jason810496 commented on code in PR #69308: URL: https://github.com/apache/airflow/pull/69308#discussion_r3519384440
########## dev/skill-evals/cases/newsfragment.yaml: ########## @@ -0,0 +1,64 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +--- +# Cases from real PRs where agents created newsfragments that +# reviewers asked to remove. These are the ambiguous boundary +# cases — the changes look like they could be user-facing. + +- description: "Scheduler bugfix (#64322): no newsfragment" + vars: + request: | + I fixed the scheduler to skip asset-triggered Dags that don't + have a SerializedDagModel yet. Without this fix, premature + asset-triggered DagRuns could be created during parsing. + The fix is in airflow-core/src/airflow/models/dag.py. + Should I create a newsfragment? + assert: + - type: javascript + value: 'output.should_create === false' + +- description: "i18n cache busting (#65720): no newsfragment" + vars: + request: | + I fixed stale translation files after Airflow upgrades by + appending the version number to the i18n file load path. + The fix is in airflow-core/src/airflow/ui/src/i18n/config.ts. + Should I create a newsfragment? + assert: + - type: javascript + value: 'output.should_create === false' + +- description: "API query optimization (#66696): no newsfragment" + vars: + request: | + I replaced COALESCE with index-friendly OR conditions in the + datetime range filters in airflow-core API to improve query + performance. No behavior change for API consumers. + Should I create a newsfragment? + assert: + - type: javascript + value: 'output.should_create === false' + +- description: "Provider bug fix (#67333): no newsfragment" + vars: + request: | + I fixed a monitoring-pod leak in KubernetesJobOperator. + The fix is in providers/cncf/kubernetes/. + Should I create a newsfragment? + assert: + - type: javascript + value: 'output.should_create === false' Review Comment: Do we need to add the positive test case? Here're some cases that I thought: Airflow security boundary changes, the recent `coordinator` interface, the recent TestConnection change (execute the TestConnection workload on worker instead of directly being executed on API server). (I will pasted the corresponding PR links here). ########## dev/skill-evals/eval.py: ########## @@ -0,0 +1,350 @@ +#!/usr/bin/env python3 +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +# /// script +# requires-python = ">=3.9" +# /// +"""Airflow skill-eval harness. + +Test whether AGENTS.md guidance affects agent decisions by comparing +arms with and without the guidance. Each arm is a git worktree of the +real repo — the agent sees actual source files. + +Usage: + uv run dev/skill-evals/eval.py Test AGENTS.md changes + uv run dev/skill-evals/eval.py --full Add baseline arm (no AGENTS.md) + uv run dev/skill-evals/eval.py --repeat 3 Reduce nondeterminism + +Authentication: Claude Code session (claude /login) or ANTHROPIC_API_KEY. +""" + +from __future__ import annotations + +import os +import shutil +import subprocess +import sys +import tempfile +from pathlib import Path + +PROMPTFOO_VERSION = "0.121.17" + +REPO_ROOT = Path(__file__).resolve().parent.parent.parent +SCRIPT_DIR = Path(__file__).resolve().parent +AGENTS_SRC = REPO_ROOT / "AGENTS.md" + +OUTPUT_SCHEMA = """\ + output_format: + type: json_schema + schema: + type: object + required: [should_create, rationale] + additionalProperties: false + properties: + should_create: + type: boolean + type: + type: string + enum: [bugfix, feature, improvement, doc, misc, significant] + rationale: + type: string""" + + +def run(cmd: list[str], **kwargs) -> subprocess.CompletedProcess[str]: + return subprocess.run(cmd, capture_output=True, text=True, check=False, **kwargs) + + +def check_prerequisites() -> None: + if not shutil.which("node"): + print("Error: Node.js not found. Install Node.js >=22.22.0", file=sys.stderr) + sys.exit(1) + if not shutil.which("npx"): + print("Error: npx not found.", file=sys.stderr) + sys.exit(1) + + sdk_dir = Path.home() / ".promptfoo-sdk" / "node_modules" / "@anthropic-ai" / "claude-agent-sdk" + if not sdk_dir.is_dir(): + print("Error: Claude Agent SDK not found. Run:", file=sys.stderr) + print( + " mkdir -p ~/.promptfoo-sdk && cd ~/.promptfoo-sdk" Review Comment: Small nits: that currently all the user generated content should be located under the `/files` artifact directory. This convention is also introduced in AGENTS.md recently by TP (might also be the classic case for eval?) (I will pasted the corresponding PR links here). ########## dev/skill-evals/README.md: ########## @@ -0,0 +1,129 @@ +<!-- + Licensed to the Apache Software Foundation (ASF) under one + or more contributor license agreements. See the NOTICE file + distributed with this work for additional information + regarding copyright ownership. The ASF licenses this file + to you under the Apache License, Version 2.0 (the + "License"); you may not use this file except in compliance + with the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, + software distributed under the License is distributed on an + "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + KIND, either express or implied. See the License for the + specific language governing permissions and limitations + under the License. +--> + +<!-- START doctoc generated TOC please keep comment here to allow auto update --> +<!-- DON'T EDIT THIS SECTION, INSTEAD RE-RUN doctoc TO UPDATE --> +**Table of Contents** *generated with [DocToc](https://github.com/thlorenz/doctoc)* + +- [Skill-Eval Harness](#skill-eval-harness) + - [Prerequisites](#prerequisites) + - [One-time setup](#one-time-setup) + - [Usage](#usage) + - [Adding cases](#adding-cases) + - [How it works](#how-it-works) + - [Prek hook](#prek-hook) + +<!-- END doctoc generated TOC please keep comment here to allow auto update --> + +# Skill-Eval Harness + +Test whether changes to `AGENTS.md` break or improve agent decisions. +The harness compares the `main` branch version against your working +tree, running the same cases against both and reporting the diff. + +Each arm is a **git worktree** of the real repo — the agent sees +actual source files (`pyproject.toml`, directory structure, etc.). +The only difference between arms is which `AGENTS.md` is present. + +The agent reads guidance through the `CLAUDE.md → AGENTS.md` symlink, +so the harness verifies that symlink exists (in the working tree and +on the base branch) before running and aborts if it is broken — a +regular-file CLAUDE.md would make every arm read identical guidance. + +## Prerequisites + +- **Node.js >=22.22.0** — check with `node --version` +- **Authentication** (one of): + - Claude Code session (`claude /login`) — Pro/Max subscription + - `ANTHROPIC_API_KEY` environment variable — API credits + +## One-time setup + +Install the Claude Agent SDK (promptfoo needs it at runtime): + +```bash +mkdir -p ~/.promptfoo-sdk && cd ~/.promptfoo-sdk && npm init -y && npm install @anthropic-ai/claude-agent-sdk +``` + +## Usage + +```bash +# Check for regressions after editing AGENTS.md: +uv run dev/skill-evals/eval.py + +# Repeat each case to reduce nondeterminism: +uv run dev/skill-evals/eval.py --repeat 3 + +# Add baseline arm (no AGENTS.md) to measure raw model capability: +uv run dev/skill-evals/eval.py --full + +# Test a skill alongside AGENTS.md (not combinable with --full — the +# baseline arm has no skill, so the skill-used assertion would always fail): +SKILL_NAME=airflow-contribution uv run dev/skill-evals/eval.py + +# Use a cheaper model for fast iteration: +MODEL=claude-haiku-4-5-20251001 uv run dev/skill-evals/eval.py + +# Disable cache (force fresh LLM calls): +uv run dev/skill-evals/eval.py --no-cache + +# View results in browser (use the pinned version printed by eval.py): +npx [email protected] view Review Comment: Then perhaps we can use the another stage: manual prek hook to show the result (to manage the node deps by prek). ########## scripts/ci/prek/check_eval_reminder.py: ########## @@ -0,0 +1,58 @@ +#!/usr/bin/env python +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +"""Remind contributors to run skill-eval when guidance files change. + +This hook prints a warning when AGENTS.md or SKILL.md is staged for +commit. It always exits 0 — it never blocks the commit. +""" Review Comment: Nice, perhaps we can add the slack bot as follow-up to run this as CI monthly then post the message to the `#internal-ci-cd-channel` then someone will eval for sure! ########## dev/skill-evals/eval.py: ########## @@ -0,0 +1,350 @@ +#!/usr/bin/env python3 +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +# /// script +# requires-python = ">=3.9" +# /// +"""Airflow skill-eval harness. + +Test whether AGENTS.md guidance affects agent decisions by comparing +arms with and without the guidance. Each arm is a git worktree of the +real repo — the agent sees actual source files. + +Usage: + uv run dev/skill-evals/eval.py Test AGENTS.md changes + uv run dev/skill-evals/eval.py --full Add baseline arm (no AGENTS.md) + uv run dev/skill-evals/eval.py --repeat 3 Reduce nondeterminism + +Authentication: Claude Code session (claude /login) or ANTHROPIC_API_KEY. +""" + +from __future__ import annotations + +import os +import shutil +import subprocess +import sys +import tempfile +from pathlib import Path + +PROMPTFOO_VERSION = "0.121.17" + +REPO_ROOT = Path(__file__).resolve().parent.parent.parent +SCRIPT_DIR = Path(__file__).resolve().parent +AGENTS_SRC = REPO_ROOT / "AGENTS.md" + +OUTPUT_SCHEMA = """\ + output_format: + type: json_schema + schema: + type: object + required: [should_create, rationale] + additionalProperties: false + properties: + should_create: + type: boolean + type: + type: string + enum: [bugfix, feature, improvement, doc, misc, significant] + rationale: + type: string""" + + +def run(cmd: list[str], **kwargs) -> subprocess.CompletedProcess[str]: + return subprocess.run(cmd, capture_output=True, text=True, check=False, **kwargs) + + +def check_prerequisites() -> None: + if not shutil.which("node"): + print("Error: Node.js not found. Install Node.js >=22.22.0", file=sys.stderr) + sys.exit(1) + if not shutil.which("npx"): + print("Error: npx not found.", file=sys.stderr) + sys.exit(1) + + sdk_dir = Path.home() / ".promptfoo-sdk" / "node_modules" / "@anthropic-ai" / "claude-agent-sdk" + if not sdk_dir.is_dir(): + print("Error: Claude Agent SDK not found. Run:", file=sys.stderr) + print( + " mkdir -p ~/.promptfoo-sdk && cd ~/.promptfoo-sdk" + " && npm init -y && npm install @anthropic-ai/claude-agent-sdk", + file=sys.stderr, + ) + sys.exit(1) + + +def check_claude_md_symlink(base_branch: str) -> None: + """Verify CLAUDE.md is a symlink to AGENTS.md, in the working tree and on the base branch. + + The Claude Agent SDK reads CLAUDE.md. Swapping AGENTS.md between arms only + affects the agent while CLAUDE.md resolves to it — if CLAUDE.md ever becomes + a regular file, every arm would read identical guidance and the eval would + silently measure nothing. + """ + link = REPO_ROOT / "CLAUDE.md" + working_ok = link.is_symlink() and os.readlink(link) == "AGENTS.md" + tree_entry = run(["git", "-C", str(REPO_ROOT), "ls-tree", base_branch, "CLAUDE.md"]).stdout + target = run(["git", "-C", str(REPO_ROOT), "show", f"{base_branch}:CLAUDE.md"]).stdout + branch_ok = tree_entry.startswith("120000") and target == "AGENTS.md" + if not (working_ok and branch_ok): + print( + "Error: CLAUDE.md must be a symlink to AGENTS.md (in the working tree" + f" and on '{base_branch}'). The eval swaps AGENTS.md between arms and" + " relies on the agent reading it through that symlink.", + file=sys.stderr, + ) + sys.exit(1) + + +def resolve_base_branch() -> str: + result = run(["git", "-C", str(REPO_ROOT), "rev-parse", "--verify", "main"]) + if result.returncode == 0: + return "main" + branch = run(["git", "-C", str(REPO_ROOT), "rev-parse", "--abbrev-ref", "HEAD"]).stdout.strip() + print(f"Warning: 'main' not found, using current branch '{branch}' as base") + return branch + + +def git_show_file(base_branch: str, path: str, dest: Path) -> bool: + result = run(["git", "-C", str(REPO_ROOT), "show", f"{base_branch}:{path}"]) + if result.returncode != 0: + return False + dest.write_text(result.stdout) + return True + + +def create_worktree( + work_dir: Path, + name: str, + base_branch: str, + agents_file: Path | None, + worktrees: list[Path], + skill_name: str | None = None, + skill_file: Path | None = None, +) -> Path: + """Create a git worktree arm with the specified AGENTS.md and optional SKILL.md.""" + wt_dir = work_dir / name + result = run( + [ + "git", + "-C", + str(REPO_ROOT), + "worktree", + "add", + "--quiet", + "--detach", + str(wt_dir), + base_branch, + ] + ) + if result.returncode != 0: + print( + f"Error: 'git worktree add' failed for arm '{name}':\n{result.stderr.strip()}", + file=sys.stderr, + ) + sys.exit(1) + # Register before mutating the checkout so cleanup covers this worktree + # even if a later step in this function fails. + worktrees.append(wt_dir) + + if agents_file is None: + # Baseline arm: remove all guidance. CLAUDE.md is a symlink to + # AGENTS.md — drop it too so the SDK finds no dangling link. + (wt_dir / "AGENTS.md").unlink(missing_ok=True) + (wt_dir / "CLAUDE.md").unlink(missing_ok=True) + else: + shutil.copy2(agents_file, wt_dir / "AGENTS.md") + + if skill_name and skill_file: + skill_dir = wt_dir / ".agents" / "skills" / skill_name + skill_dir.mkdir(parents=True, exist_ok=True) + shutil.copy2(skill_file, skill_dir / "SKILL.md") + + return wt_dir + + +def remove_worktree(wt_dir: Path) -> None: + run(["git", "-C", str(REPO_ROOT), "worktree", "remove", "--force", str(wt_dir)]) + + +def add_provider( + config_lines: list[str], + label: str, + working_dir: Path, + model: str, + skill_name: str | None = None, +) -> None: + config_lines.append(" - id: anthropic:claude-agent-sdk") + config_lines.append(f" label: {label}") + config_lines.append(" config:") + config_lines.append(f" model: {model}") + config_lines.append(" apiKeyRequired: false") + config_lines.append(" setting_sources: ['project']") + config_lines.append(" append_allowed_tools: ['Read', 'Grep', 'Glob']") + config_lines.append(f" working_dir: {working_dir}") + if skill_name: + config_lines.append(f" skills: ['{skill_name}']") + config_lines.append(OUTPUT_SCHEMA) + config_lines.append("") + + +def main() -> int: + check_prerequisites() + + model = os.environ.get("MODEL", "claude-sonnet-4-6") + skill_name = os.environ.get("SKILL_NAME") + skill_src = None + if skill_name: + skill_src = REPO_ROOT / ".agents" / "skills" / skill_name / "SKILL.md" + if not skill_src.is_file(): + print(f"Error: {skill_src} not found", file=sys.stderr) + return 1 + + # Parse flags + full_mode = "--full" in sys.argv + promptfoo_args = [a for a in sys.argv[1:] if a != "--full"] + + if full_mode and skill_name: + print( + "Error: --full cannot be combined with SKILL_NAME — the baseline arm has" + " no skill, so the 'skill-used' assertion would always fail on it.", + file=sys.stderr, + ) + return 1 + + base_branch = resolve_base_branch() + check_claude_md_symlink(base_branch) + + # Temp dir for config and worktrees + work_dir = Path(tempfile.mkdtemp()) + worktrees: list[Path] = [] + + try: + # Symlink node_modules for promptfoo SDK resolution + sdk_modules = Path.home() / ".promptfoo-sdk" / "node_modules" + (work_dir / "node_modules").symlink_to(sdk_modules) + + # Extract main-branch AGENTS.md + main_agents = work_dir / "main-agents.md" + if not git_show_file(base_branch, "AGENTS.md", main_agents): + shutil.copy2(AGENTS_SRC, main_agents) + print(f"Warning: AGENTS.md not found on {base_branch} — main arm uses working tree copy") + + main_skill = None + if skill_name and skill_src: + main_skill = work_dir / "main-skill.md" + skill_git_path = f".agents/skills/{skill_name}/SKILL.md" + if not git_show_file(base_branch, skill_git_path, main_skill): + shutil.copy2(skill_src, main_skill) + print(f"Warning: SKILL.md not found on {base_branch} — main arm uses working tree copy") + + # Detect changes to decide which arms to build + agents_changed = run(["diff", "-q", str(main_agents), str(AGENTS_SRC)]).returncode != 0 + skill_changed = False + if skill_name and main_skill and skill_src: + skill_changed = run(["diff", "-q", str(main_skill), str(skill_src)]).returncode != 0 + need_working = agents_changed or skill_changed + + # Assemble arms. Prune first to self-heal stale worktree registrations + # left behind by a previously interrupted run. + print("Assembling arms (git worktrees) ...") + run(["git", "-C", str(REPO_ROOT), "worktree", "prune"]) + + arm_main = create_worktree( + work_dir, "main", base_branch, main_agents, worktrees, skill_name, main_skill + ) + + arm_working = None + if need_working: + arm_working = create_worktree( + work_dir, "working", base_branch, AGENTS_SRC, worktrees, skill_name, skill_src + ) + else: + print(" AGENTS.md unchanged — skipping working arm") + + arm_baseline = None + if full_mode: + arm_baseline = create_worktree(work_dir, "baseline", base_branch, None, worktrees) + + # Generate config + config_lines: list[str] = [] + config_lines.append("prompts:") + config_lines.append(" - '{{request}}'") + config_lines.append("") + config_lines.append("providers:") + + arm_count = 0 + skill_for_provider = skill_name if skill_name else None + + add_provider(config_lines, "main", arm_main, model, skill_for_provider) + arm_count += 1 + + if arm_working: + add_provider(config_lines, "working", arm_working, model, skill_for_provider) + arm_count += 1 + + if full_mode and arm_baseline: + add_provider(config_lines, "baseline", arm_baseline, model) + arm_count += 1 + + # Default test config + config_lines.append("defaultTest:") + config_lines.append(" options:") + config_lines.append(" disableVarExpansion: true") + if skill_name: + config_lines.append(" assert:") + config_lines.append(" - type: skill-used") + config_lines.append(f" value: {skill_name}") + config_lines.append("") Review Comment: Not sure could it be better to construct using dict then serialize at the end of the script as yaml? (Additionally, we can use TypeDict to represent those hierarchy structurally. ########## dev/skill-evals/eval.py: ########## @@ -0,0 +1,350 @@ +#!/usr/bin/env python3 +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +# /// script +# requires-python = ">=3.9" +# /// +"""Airflow skill-eval harness. + +Test whether AGENTS.md guidance affects agent decisions by comparing +arms with and without the guidance. Each arm is a git worktree of the +real repo — the agent sees actual source files. + +Usage: + uv run dev/skill-evals/eval.py Test AGENTS.md changes + uv run dev/skill-evals/eval.py --full Add baseline arm (no AGENTS.md) + uv run dev/skill-evals/eval.py --repeat 3 Reduce nondeterminism + +Authentication: Claude Code session (claude /login) or ANTHROPIC_API_KEY. +""" + +from __future__ import annotations + +import os +import shutil +import subprocess +import sys +import tempfile +from pathlib import Path + +PROMPTFOO_VERSION = "0.121.17" + +REPO_ROOT = Path(__file__).resolve().parent.parent.parent +SCRIPT_DIR = Path(__file__).resolve().parent +AGENTS_SRC = REPO_ROOT / "AGENTS.md" + +OUTPUT_SCHEMA = """\ + output_format: + type: json_schema + schema: + type: object + required: [should_create, rationale] + additionalProperties: false + properties: + should_create: + type: boolean + type: + type: string + enum: [bugfix, feature, improvement, doc, misc, significant] + rationale: + type: string""" + + +def run(cmd: list[str], **kwargs) -> subprocess.CompletedProcess[str]: + return subprocess.run(cmd, capture_output=True, text=True, check=False, **kwargs) + + +def check_prerequisites() -> None: + if not shutil.which("node"): + print("Error: Node.js not found. Install Node.js >=22.22.0", file=sys.stderr) + sys.exit(1) + if not shutil.which("npx"): + print("Error: npx not found.", file=sys.stderr) + sys.exit(1) Review Comment: I wonder could we ensure these system level deps be handled by prek hook level. IIRC, we're able to set system deps on the pre-commit config. Then user don't need to install the node runtime themself. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
