jason810496 commented on code in PR #70120:
URL: https://github.com/apache/airflow/pull/70120#discussion_r3742625090
##########
.pre-commit-config.yaml:
##########
@@ -1034,6 +1034,27 @@ repos:
pass_filenames: false
require_serial: true
verbose: true
+ - id: run-skill-eval-codex
+ name: Run skill-eval against AGENTS.md and cases (Codex runtime)
+ # Kept separate from run-skill-eval so only contributors who actually
run
+ # the Codex arm build an env carrying the Codex CLI binary. promptfoo
+ # bundles its own @openai/codex-sdk, but that build ships a binary
whose
+ # Developer ID certificate Apple has revoked — macOS kills it on exec —
+ # so a notarized version has to be installed explicitly.
+ entry: env AGENT_RUNTIME=codex ./dev/skill-evals/eval.py
+ language: node
+ language_version: '22.22.0'
+ additional_dependencies:
+ - '[email protected]'
+ - '@openai/[email protected]'
+ stages: ['manual']
Review Comment:
`manual` is also the stage used by the repository-wide bulk command, so this
hook still matches every `AGENTS.md` / `dev/skill-evals/` change. `prek` will
therefore provision the ~300 MB Codex environment and then require Codex
authentication for contributors who intended to run only Claude.
After rebasing onto `main`, please extend the existing bulk-manual skip list
in the generated command source and its consumers:
```diff
-prek run --from-ref <target_branch> --stage manual --skip
compile-ui-assets-dev --skip view-skill-eval
+prek run --from-ref <target_branch> --stage manual --skip
compile-ui-assets-dev --skip view-skill-eval --skip run-skill-eval-codex
```
That means updating `contributing-docs/08_static_code_checks.rst`, the
generated command in `AGENTS.md`, and the pre-push checklist command in
`AGENTS.md`. The explicit `prek run run-skill-eval-codex ...` path documented
in `dev/skill-evals/README.md` will remain opt-in.
##########
scripts/tests/ci/prek/test_skill_eval.py:
##########
@@ -0,0 +1,174 @@
+# 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.
+from __future__ import annotations
+
+import importlib.util
+from pathlib import Path
+from types import SimpleNamespace
+
+import pytest
+
+REPO_ROOT = Path(__file__).resolve().parents[4]
+MODULE_PATH = REPO_ROOT / "dev" / "skill-evals" / "eval.py"
+
+
[email protected](scope="module")
+def skill_eval_module():
+ spec = importlib.util.spec_from_file_location("skill_eval", MODULE_PATH)
+ assert spec and spec.loader
+ module = importlib.util.module_from_spec(spec)
+ spec.loader.exec_module(module)
+ return module
+
+
[email protected](
+ ("configured", "expected"),
+ [
+ pytest.param(None, "claude", id="default"),
+ pytest.param("Claude", "claude", id="case-insensitive"),
+ pytest.param("codex", "codex", id="codex"),
+ ],
+)
+def test_get_runtime(monkeypatch, skill_eval_module, configured, expected):
+ if configured is None:
+ monkeypatch.delenv("AGENT_RUNTIME", raising=False)
+ else:
+ monkeypatch.setenv("AGENT_RUNTIME", configured)
+
+ assert skill_eval_module.get_runtime() == expected
+
+
+def test_get_runtime_rejects_unknown_runtime(monkeypatch, skill_eval_module):
+ monkeypatch.setenv("AGENT_RUNTIME", "unknown")
+
+ with pytest.raises(ValueError, match="claude, codex"):
+ skill_eval_module.get_runtime()
+
+
+def test_build_codex_provider(skill_eval_module, tmp_path):
+ working_dir = tmp_path / "worktree"
+
+ provider = skill_eval_module.build_codex_provider(
+ "working", working_dir, "gpt-test", detect_skill_usage=True
+ )
+
+ assert provider == {
+ "id": "openai:codex-sdk",
+ "label": "working",
+ "config": {
+ "approval_policy": "never",
+ "cli_config": {
+ "history": {"persistence": "none"},
+ "project_doc_max_bytes":
skill_eval_module.CODEX_PROJECT_DOC_MAX_BYTES,
+ },
+ "enable_streaming": True,
+ "model": "gpt-test",
+ "network_access_enabled": False,
+ "output_schema": skill_eval_module.OUTPUT_FORMAT["schema"],
+ "sandbox_mode": "read-only",
+ "web_search_mode": "disabled",
+ "working_dir": str(working_dir),
+ },
+ "transform": "JSON.parse(output)",
+ }
+
+
+def test_output_schema_requires_every_property(skill_eval_module):
+ schema = skill_eval_module.OUTPUT_FORMAT["schema"]
+
+ assert set(schema["required"]) == set(schema["properties"])
+
+
+def test_build_codex_provider_uses_runtime_default_model(skill_eval_module,
tmp_path):
+ provider = skill_eval_module.build_codex_provider("main", tmp_path /
"worktree", None)
+
+ assert "model" not in provider["config"]
+ assert "enable_streaming" not in provider["config"]
+
+
+def test_build_claude_provider_keeps_existing_configuration(skill_eval_module,
tmp_path):
+ provider = skill_eval_module.build_provider("main", tmp_path,
"claude-test", "example-skill")
+
+ assert provider["id"] == "anthropic:claude-agent-sdk"
+ assert provider["label"] == "main"
+ assert provider["config"] == {
+ "apiKeyRequired": False,
+ "append_allowed_tools": ["Read", "Grep", "Glob"],
+ "model": "claude-test",
+ "output_format": skill_eval_module.OUTPUT_FORMAT,
+ "setting_sources": ["project"],
+ "skills": ["example-skill"],
+ "working_dir": str(tmp_path),
+ }
+
+
Review Comment:
This function exercises the pre-existing Claude branch unchanged by this PR.
It still passes if the production change is reverted, so it conflicts with the
repository requirement that each added test fail without the PR. Please remove
it; the Codex/runtime tests above cover the new behavior.
```suggestion
```
--
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]
