From 4cf1eacdd4ec23a81df4a6b2cbbd4ad64cf587c6 Mon Sep 17 00:00:00 2001 From: YMuskrat <101849520+YMuskrat@users.noreply.github.com> Date: Mon, 12 Oct 2026 12:56:51 +0100 Subject: [PATCH 1/8] fix(tier3): exclude failed skill reads from execution credit Signed-off-by: YMuskrat --- .../tier3/eval_core/atif_helpers.py | 4 +++- src/skillevaluator/tier3/eval_core/checks.py | 13 +++++++------ .../eval_core/codex_tool_call_normalizer.py | 18 ++++++++++++++++++ .../tier3/eval_core/log_converters.py | 17 +++++++++-------- 4 files changed, 37 insertions(+), 15 deletions(-) diff --git a/src/skillevaluator/tier3/eval_core/atif_helpers.py b/src/skillevaluator/tier3/eval_core/atif_helpers.py index 6dbf126b..480918c8 100644 --- a/src/skillevaluator/tier3/eval_core/atif_helpers.py +++ b/src/skillevaluator/tier3/eval_core/atif_helpers.py @@ -17,7 +17,7 @@ from typing import Any from skillevaluator.evidence import evidence_ref_identity -from skillevaluator.tier3.eval_core.codex_tool_call_normalizer import atif_content_text +from skillevaluator.tier3.eval_core.codex_tool_call_normalizer import atif_content_text, normalized_tool_call_is_error from skillevaluator.tier3.eval_core.codex_tool_call_normalizer import ( iter_normalized_tool_calls as iter_tool_calls, ) @@ -1053,6 +1053,8 @@ def extract_tool_calls_as_dicts(traj: dict[str, Any]) -> list[dict[str, Any]]: "action_input": tc.get("arguments") or {}, "observation": _tool_call_observation(step, tc), } + if normalized_tool_call_is_error(step, tc): + call["is_error"] = True if status := tc.get("_atif_normalization_status"): call["normalization_status"] = status if status := tc.get("_atif_observation_status"): diff --git a/src/skillevaluator/tier3/eval_core/checks.py b/src/skillevaluator/tier3/eval_core/checks.py index 4ad180fc..8e4a783f 100644 --- a/src/skillevaluator/tier3/eval_core/checks.py +++ b/src/skillevaluator/tier3/eval_core/checks.py @@ -2018,7 +2018,7 @@ def check_activation( } # Check 2: read_file / read with SKILL.md in path - read_calls = [tc for tc in tool_calls if "read" in tc["action"].lower()] + read_calls = [tc for tc in tool_calls if "read" in tc["action"].lower() and tc.get("is_error") is not True] for call in read_calls: path_arg = _extract_path(call) if "SKILL.md" not in path_arg: @@ -2037,7 +2037,7 @@ def check_activation( } # Check 3: shell read of SKILL.md (cat/sed/head/...) - exec_calls = [tc for tc in tool_calls if _is_execution_action(str(tc["action"]))] + exec_calls = [tc for tc in tool_calls if _is_execution_action(str(tc["action"])) and tc.get("is_error") is not True] for call in exec_calls: cmd = _command_text(call) if _cmd_reads_skill_md(cmd): @@ -2073,7 +2073,7 @@ def check_activation( has_skill_read_evidence = ("read" in action.lower() and "SKILL.md" in str(tc.get("action_input", ""))) or ( _is_execution_action(action) and _cmd_reads_skill_md(cmd) ) - if not has_skill_read_evidence: + if not has_skill_read_evidence or tc.get("is_error") is True: continue obs = str(tc.get("observation", "")).lower() if "skill.md" in obs: @@ -5107,7 +5107,8 @@ def check_workflow_order( cmd = _command_text(call) if ("read" in action and "SKILL.md" in args_str) or (_is_execution_action(action) and _cmd_reads_skill_md(cmd)): - sequence.append("read_skill") + if call.get("is_error") is not True: + sequence.append("read_skill") elif _is_execution_action(action) and cmd and "--help" not in cmd and "which " not in cmd: sequence.append("execution") @@ -5365,7 +5366,7 @@ def check_routing( ) -> dict[str, Any]: """Check the agent read only expected/allowed workspace skill docs.""" unsupported_native_codex_call = _has_unsupported_native_codex_call(tool_calls) - read_calls = [tc for tc in tool_calls if "read" in tc["action"].lower()] + read_calls = [tc for tc in tool_calls if "read" in tc["action"].lower() and tc.get("is_error") is not True] skills_read: list[str] = [] wrong_skills: list[str] = [] @@ -5398,7 +5399,7 @@ def check_routing( for call in tool_calls: action = call["action"].lower() cmd = _command_text(call) - if not (_is_execution_action(action) and _cmd_reads_skill_md(cmd)): + if call.get("is_error") is True or not (_is_execution_action(action) and _cmd_reads_skill_md(cmd)): continue skills_read.append(cmd) diff --git a/src/skillevaluator/tier3/eval_core/codex_tool_call_normalizer.py b/src/skillevaluator/tier3/eval_core/codex_tool_call_normalizer.py index e5d751b4..8fe5de88 100644 --- a/src/skillevaluator/tier3/eval_core/codex_tool_call_normalizer.py +++ b/src/skillevaluator/tier3/eval_core/codex_tool_call_normalizer.py @@ -81,6 +81,24 @@ def normalized_tool_call_observation(step: dict[str, Any], tool_call: dict[str, ) +def normalized_tool_call_is_error(step: dict[str, Any], tool_call: dict[str, Any]) -> bool: + """Preserve explicit failure metadata only for an attributable tool result.""" + if tool_call.get("_atif_observation_status") not in (None, MAPPED_OUTER_EXEC_OBSERVATION): + return False + tool_call_id = tool_call.get("tool_call_id") + for result in (step.get("observation") or {}).get("results") or []: + source_call_id = result.get("source_call_id") + if not ( + (tool_call_id and source_call_id == tool_call_id) + or (not source_call_id and tool_call.get("_atif_raw_tool_index", 0) == 0) + ): + continue + extra = result.get("extra") or {} + if isinstance(extra, dict) and extra.get("tool_result_is_error") is True: + return True + return False + + def normalized_tool_call_wrapper_observation(step: dict[str, Any], tool_call: dict[str, Any]) -> str: """Return an unattributed wrapper observation once for security scanning.""" if tool_call.get("_atif_observation_status") not in ( diff --git a/src/skillevaluator/tier3/eval_core/log_converters.py b/src/skillevaluator/tier3/eval_core/log_converters.py index 4f4812d4..4a18f274 100644 --- a/src/skillevaluator/tier3/eval_core/log_converters.py +++ b/src/skillevaluator/tier3/eval_core/log_converters.py @@ -54,7 +54,9 @@ def _stringify_tool_result_content(content: Any) -> str: return str(content) -def _attach_tool_result(steps: list[dict[str, Any]], tool_use_id: str, content_text: str) -> None: +def _attach_tool_result( + steps: list[dict[str, Any]], tool_use_id: str, content_text: str, *, is_error: bool = False +) -> None: for step in reversed(steps): if step.get("source") != "agent": continue @@ -62,12 +64,11 @@ def _attach_tool_result(steps: list[dict[str, Any]], tool_use_id: str, content_t if tc.get("tool_call_id") == tool_use_id: step.setdefault("observation", {"results": []}) step["observation"].setdefault("results", []) - step["observation"]["results"].append( - { - "source_call_id": tool_use_id, - "content": content_text, - } - ) + result: dict[str, Any] = {"source_call_id": tool_use_id, "content": content_text} + if is_error: + # Match Harbor's native Claude Code ATIF result metadata. + result["extra"] = {"tool_result_is_error": True} + step["observation"]["results"].append(result) return @@ -158,7 +159,7 @@ def synthetic_trajectory_from_claude_stream_jsonl(text: str) -> dict[str, Any] | tid = str(block.get("tool_use_id") or "") res_text = _stringify_tool_result_content(block.get("content")) if tid: - _attach_tool_result(steps, tid, res_text) + _attach_tool_result(steps, tid, res_text, is_error=block.get("is_error") is True) elif et == "result": res = evt.get("result") From 85ae1c0247389d7fb8d203bb96ab35f9f0da3120 Mon Sep 17 00:00:00 2001 From: YMuskrat <101849520+YMuskrat@users.noreply.github.com> Date: Mon, 12 Oct 2026 12:57:49 +0100 Subject: [PATCH 2/8] fix(tier3): match verifier credit to explicit skill-read results Signed-off-by: YMuskrat --- .../tier3/harbor/templates/eval.py | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/src/skillevaluator/tier3/harbor/templates/eval.py b/src/skillevaluator/tier3/harbor/templates/eval.py index 63f4ab74..7e1aeec0 100644 --- a/src/skillevaluator/tier3/harbor/templates/eval.py +++ b/src/skillevaluator/tier3/harbor/templates/eval.py @@ -76,6 +76,7 @@ def load_trajectory_with_fallback(trajectory_path, logs_dir=None): UNSUPPORTED_NATIVE_CODEX_EXEC, atif_content_text, iter_normalized_tool_calls, + normalized_tool_call_is_error, normalized_tool_call_observation, normalized_tool_call_wrapper_observation, ) @@ -86,6 +87,7 @@ def load_trajectory_with_fallback(trajectory_path, logs_dir=None): UNSUPPORTED_NATIVE_CODEX_EXEC, atif_content_text, iter_normalized_tool_calls, + normalized_tool_call_is_error, normalized_tool_call_observation, normalized_tool_call_wrapper_observation, ) @@ -445,6 +447,8 @@ def extract_tool_calls_as_dicts(traj): "action_input": tc.get("arguments") or {}, "observation": _tool_call_observation(step, tc), } + if normalized_tool_call_is_error(step, tc): + call["is_error"] = True if status := tc.get("_atif_normalization_status"): call["normalization_status"] = status if status := tc.get("_atif_observation_status"): @@ -4457,7 +4461,7 @@ def check_activation(tool_calls, expected_skill, skill_tool_names=None, acceptab "reason": reason, "details": {**_skill_match_details(expected_skill, acceptable_skills), **match}, } - read_calls = [tc for tc in tool_calls if "read" in tc["action"].lower()] + read_calls = [tc for tc in tool_calls if "read" in tc["action"].lower() and tc.get("is_error") is not True] for call in read_calls: path_arg = _extract_path(call) if "SKILL.md" not in path_arg: @@ -4475,7 +4479,7 @@ def check_activation(tool_calls, expected_skill, skill_tool_names=None, acceptab "details": {**_skill_match_details(expected_skill, acceptable_skills), **match, "path": path_arg}, } for call in tool_calls: - if _is_execution_action(call["action"]): + if _is_execution_action(call["action"]) and call.get("is_error") is not True: cmd = _command_text(call) if _cmd_reads_skill_md(cmd): match = _classify_skill_match(cmd, expected_skill, acceptable_skills, fuzzy=True) @@ -4507,7 +4511,7 @@ def check_activation(tool_calls, expected_skill, skill_tool_names=None, acceptab has_skill_read_evidence = ("read" in action.lower() and "SKILL.md" in str(tc.get("action_input", ""))) or ( _is_execution_action(action) and _cmd_reads_skill_md(cmd) ) - if not has_skill_read_evidence: + if not has_skill_read_evidence or tc.get("is_error") is True: continue obs = str(tc.get("observation", "")).lower() if "skill.md" in obs: @@ -7524,7 +7528,8 @@ def check_workflow_order(tool_calls, skill_tool_names=None, expected_skill=None) args_str = str(call.get("action_input", "")) cmd = _command_text(call) if ("read" in action and "SKILL.md" in args_str) or (_is_execution_action(action) and _cmd_reads_skill_md(cmd)): - sequence.append("read_skill") + if call.get("is_error") is not True: + sequence.append("read_skill") elif _is_execution_action(action) and cmd and "--help" not in cmd and "which " not in cmd: sequence.append("execution") if not sequence: @@ -7607,7 +7612,7 @@ def check_routing( acceptable_skills=None, ): unsupported_native_codex_call = _has_unsupported_native_codex_call(tool_calls) - read_calls = [tc for tc in tool_calls if "read" in tc["action"].lower()] + read_calls = [tc for tc in tool_calls if "read" in tc["action"].lower() and tc.get("is_error") is not True] skills_read, wrong_skills = [], [] matched_expected = False matched_alternate = False @@ -7635,7 +7640,7 @@ def check_routing( for call in tool_calls: action = call["action"].lower() cmd = _command_text(call) - if not (_is_execution_action(action) and _cmd_reads_skill_md(cmd)): + if call.get("is_error") is True or not (_is_execution_action(action) and _cmd_reads_skill_md(cmd)): continue skills_read.append(cmd) skill_name = _skill_name_from_ref(cmd) From e95b48862169c58c1fa56698e1aab038b35b9659 Mon Sep 17 00:00:00 2001 From: YMuskrat <101849520+YMuskrat@users.noreply.github.com> Date: Mon, 12 Oct 2026 12:59:12 +0100 Subject: [PATCH 3/8] test(tier3): cover failed skill reads, retries and result ownership Signed-off-by: YMuskrat --- tests/tier3/test_failed_skill_reads.py | 153 +++++++++++++++++++++++++ 1 file changed, 153 insertions(+) create mode 100644 tests/tier3/test_failed_skill_reads.py diff --git a/tests/tier3/test_failed_skill_reads.py b/tests/tier3/test_failed_skill_reads.py new file mode 100644 index 00000000..04a2e925 --- /dev/null +++ b/tests/tier3/test_failed_skill_reads.py @@ -0,0 +1,153 @@ +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +"""Failed tool results cannot prove a successful SKILL.md read.""" + +from __future__ import annotations + +import json +from types import ModuleType + +import pytest + +from skillevaluator.tier3.eval_core import atif_helpers, checks +from skillevaluator.tier3.eval_core.log_converters import synthetic_trajectory_from_claude_stream_jsonl + + +@pytest.fixture(params=("host", "verifier")) +def grader(request: pytest.FixtureRequest, harbor_eval_template: ModuleType) -> tuple[ModuleType, ModuleType]: + if request.param == "host": + return atif_helpers, checks + return harbor_eval_template, harbor_eval_template + + +def _claude_read(*, is_error: bool, content: str) -> dict: + events = [ + { + "type": "assistant", + "message": { + "content": [ + { + "type": "tool_use", + "id": "read-1", + "name": "Read", + "input": {"file_path": "/skills/demo-skill/SKILL.md"}, + } + ] + }, + }, + { + "type": "user", + "message": { + "content": [ + { + "type": "tool_result", + "tool_use_id": "read-1", + "is_error": is_error, + "content": content, + } + ] + }, + }, + ] + trajectory = synthetic_trajectory_from_claude_stream_jsonl("\n".join(json.dumps(event) for event in events)) + assert trajectory is not None + return trajectory + + +def test_claude_failed_read_cannot_receive_perfect_execution_credit(grader: tuple[ModuleType, ModuleType]) -> None: + extractor, scorer = grader + trajectory = _claude_read(is_error=True, content="Error: File does not exist: /skills/demo-skill/SKILL.md") + calls = extractor.extract_tool_calls_as_dicts(trajectory) + + assert calls[0]["is_error"] is True + execution = scorer.score_skill_execution(calls, "demo-skill") + assert execution["details"]["activation"]["score"] == 0.0 + assert execution["details"]["workflow_order"]["score"] == 0.0 + assert execution["score"] < 1.0 + assert scorer.check_routing(calls, "demo-skill")["score"] == 0.0 + + +def test_successful_skill_content_can_discuss_errors(grader: tuple[ModuleType, ModuleType]) -> None: + extractor, scorer = grader + calls = extractor.extract_tool_calls_as_dicts( + _claude_read(is_error=False, content="# Demo skill\nError: File does not exist. Handle this by retrying.") + ) + + assert scorer.score_skill_execution(calls, "demo-skill")["score"] == 1.0 + assert scorer.check_routing(calls, "demo-skill")["score"] == 1.0 + + +@pytest.mark.parametrize("action", ("Read", "Bash")) +def test_failed_read_does_not_satisfy_workflow_before_execution( + grader: tuple[ModuleType, ModuleType], + action: str, +) -> None: + _, scorer = grader + read = { + "action": action, + "action_input": {"file_path": "/skills/demo-skill/SKILL.md"} + if action == "Read" + else {"command": "cat /skills/demo-skill/SKILL.md"}, + "observation": "Unable to load /skills/demo-skill/SKILL.md", + "is_error": True, + } + execute = {"action": "Bash", "action_input": {"command": "python scripts/run.py"}, "observation": "ok"} + + assert scorer.check_activation([read], "demo-skill")["score"] == 0.0 + assert scorer.check_routing([read], "demo-skill")["score"] == 0.0 + assert scorer.check_workflow_order([read, execute], expected_skill="demo-skill")["score"] == 0.0 + retry = {**read, "observation": "# Demo skill", "is_error": False} + assert scorer.check_workflow_order([read, retry, execute], expected_skill="demo-skill")["score"] == 1.0 + assert scorer.check_routing([read, retry], "demo-skill")["score"] == 1.0 + + +def test_native_atif_error_is_matched_to_its_own_call(grader: tuple[ModuleType, ModuleType]) -> None: + from harbor.models.trajectories.trajectory import Trajectory + + extractor, scorer = grader + trajectory = Trajectory.model_validate( + { + "schema_version": "ATIF-v1.7", + "agent": {"name": "claude-code", "version": "test"}, + "steps": [ + { + "step_id": 1, + "source": "agent", + "message": "", + "tool_calls": [ + { + "tool_call_id": "wrong", + "function_name": "Read", + "arguments": {"file_path": "/skills/wrong/SKILL.md"}, + }, + { + "tool_call_id": "right", + "function_name": "Read", + "arguments": {"file_path": "/skills/demo-skill/SKILL.md"}, + }, + ], + "observation": { + "results": [ + { + "source_call_id": "right", + "content": "# Demo skill", + "extra": {"tool_result_is_error": False}, + }, + { + "source_call_id": "wrong", + "content": "Missing file", + "extra": {"tool_result_is_error": True}, + }, + ] + }, + } + ], + } + ).to_json_dict() + calls = extractor.extract_tool_calls_as_dicts(trajectory) + + assert calls[0]["is_error"] is True + assert not calls[1].get("is_error") + assert scorer.score_skill_execution(calls, "demo-skill")["score"] == 1.0 + assert scorer.check_routing(calls, "demo-skill")["score"] == 1.0 From 6afe48963da80e5b54a6478e1ebb1d3197ef131f Mon Sep 17 00:00:00 2001 From: YMuskrat <101849520+YMuskrat@users.noreply.github.com> Date: Mon, 12 Oct 2026 12:59:46 +0100 Subject: [PATCH 4/8] docs: note failed-read grading correction Signed-off-by: YMuskrat --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7b35b49e..91cce41f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,9 @@ All notable changes to SkillEvaluator are documented in this file. ### Fixed +- Preserve Claude tool-result failure flags in native and reconstructed ATIF + trajectories. Failed SKILL.md reads no longer receive activation, workflow, + or routing credit in either the host grader or the Harbor verifier. - Brought the published documentation in line with 0.5.0. It now covers how standard grading builds judge evidence and rebuilds trajectories from OpenCode and Codex logs, structured judge output and retries, the evidence From 6a1fd839ad4ff5bde7f58f3dad895031836e8b22 Mon Sep 17 00:00:00 2001 From: YMuskrat <101849520+YMuskrat@users.noreply.github.com> Date: Tue, 13 Oct 2026 13:51:25 +0100 Subject: [PATCH 5/8] fix(tier3): handle compound shell reads and failed skill tools Signed-off-by: YMuskrat <101849520+YMuskrat@users.noreply.github.com> --- CHANGELOG.md | 7 +-- .../tier3/eval_core/atif_helpers.py | 14 +++-- src/skillevaluator/tier3/eval_core/checks.py | 31 +++++++++-- .../tier3/eval_core/log_converters.py | 13 +++-- .../tier3/harbor/templates/eval.py | 45 ++++++++++++---- tests/tier3/test_failed_skill_reads.py | 52 +++++++++++++++++++ 6 files changed, 134 insertions(+), 28 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 91cce41f..82f3d4e7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,9 +6,10 @@ All notable changes to SkillEvaluator are documented in this file. ### Fixed -- Preserve Claude tool-result failure flags in native and reconstructed ATIF - trajectories. Failed SKILL.md reads no longer receive activation, workflow, - or routing credit in either the host grader or the Harbor verifier. +- Preserve explicit Claude and OpenCode read failure metadata in reconstructed + ATIF trajectories. Failed isolated SKILL.md reads and failed native Skill + calls lose credit; compound shell failures remain ambiguous and retain + prior credit in host and verifier grading. - Brought the published documentation in line with 0.5.0. It now covers how standard grading builds judge evidence and rebuilds trajectories from OpenCode and Codex logs, structured judge output and retries, the evidence diff --git a/src/skillevaluator/tier3/eval_core/atif_helpers.py b/src/skillevaluator/tier3/eval_core/atif_helpers.py index 480918c8..3732ea67 100644 --- a/src/skillevaluator/tier3/eval_core/atif_helpers.py +++ b/src/skillevaluator/tier3/eval_core/atif_helpers.py @@ -53,11 +53,15 @@ def get_all_tool_calls(traj: dict[str, Any]) -> list[dict[str, Any]]: def get_skill_tool_calls(traj: dict[str, Any]) -> list[str]: """Get skill names from Claude Code's native ``Skill`` tool invocations.""" skills: list[str] = [] - for tc in get_all_tool_calls(traj): - if tc["fn"].lower() == "skill": - name = tc["args"].get("skill", tc["args"].get("name", "")) - if name: - skills.append(str(name)) + for step, tc in iter_tool_calls(traj): + if (tc.get("function_name") or "").lower() != "skill": + continue + if normalized_tool_call_is_error(step, tc): + continue + args = tc.get("arguments") or {} + name = args.get("skill", args.get("name", "")) + if name: + skills.append(str(name)) return skills diff --git a/src/skillevaluator/tier3/eval_core/checks.py b/src/skillevaluator/tier3/eval_core/checks.py index 8e4a783f..7592edf4 100644 --- a/src/skillevaluator/tier3/eval_core/checks.py +++ b/src/skillevaluator/tier3/eval_core/checks.py @@ -640,6 +640,24 @@ def _command_reads_skill_md_arg(command: list[str], cmd_idx: int, assignments: d return False +def _failed_single_skill_read(call): + """A shell failure only proves the skill read failed for an isolated read. + + Compound commands and multi-file reads may print SKILL.md successfully + before an unrelated operation fails. + """ + if call.get("is_error") is not True: + return False + tokens = _shell_tokens(_command_text(call)) + if not tokens or any(token in _SHELL_SEPARATORS for token in tokens): + return False + command = tokens[0].rsplit("/", 1)[-1].lower() + if command not in {"cat", "nl", "bat", "less", "more"}: + return False + files = [arg for arg in tokens[1:] if not arg.startswith("-")] + return len(files) == 1 and _skill_md_arg(files[0], {}) + + def _cmd_reads_skill_md(cmd) -> bool: """True if a shell command reads a SKILL.md via a file-view utility. @@ -2037,7 +2055,7 @@ def check_activation( } # Check 3: shell read of SKILL.md (cat/sed/head/...) - exec_calls = [tc for tc in tool_calls if _is_execution_action(str(tc["action"])) and tc.get("is_error") is not True] + exec_calls = [tc for tc in tool_calls if _is_execution_action(str(tc["action"])) and not _failed_single_skill_read(tc)] for call in exec_calls: cmd = _command_text(call) if _cmd_reads_skill_md(cmd): @@ -2073,7 +2091,10 @@ def check_activation( has_skill_read_evidence = ("read" in action.lower() and "SKILL.md" in str(tc.get("action_input", ""))) or ( _is_execution_action(action) and _cmd_reads_skill_md(cmd) ) - if not has_skill_read_evidence or tc.get("is_error") is True: + if not has_skill_read_evidence or ( + tc.get("is_error") is True + and (not _is_execution_action(action) or _failed_single_skill_read(tc)) + ): continue obs = str(tc.get("observation", "")).lower() if "skill.md" in obs: @@ -5107,7 +5128,9 @@ def check_workflow_order( cmd = _command_text(call) if ("read" in action and "SKILL.md" in args_str) or (_is_execution_action(action) and _cmd_reads_skill_md(cmd)): - if call.get("is_error") is not True: + if call.get("is_error") is not True or ( + _is_execution_action(action) and not _failed_single_skill_read(call) + ): sequence.append("read_skill") elif _is_execution_action(action) and cmd and "--help" not in cmd and "which " not in cmd: sequence.append("execution") @@ -5399,7 +5422,7 @@ def check_routing( for call in tool_calls: action = call["action"].lower() cmd = _command_text(call) - if call.get("is_error") is True or not (_is_execution_action(action) and _cmd_reads_skill_md(cmd)): + if not (_is_execution_action(action) and _cmd_reads_skill_md(cmd)) or _failed_single_skill_read(call): continue skills_read.append(cmd) diff --git a/src/skillevaluator/tier3/eval_core/log_converters.py b/src/skillevaluator/tier3/eval_core/log_converters.py index 4a18f274..2effe9db 100644 --- a/src/skillevaluator/tier3/eval_core/log_converters.py +++ b/src/skillevaluator/tier3/eval_core/log_converters.py @@ -565,13 +565,12 @@ def synthetic_trajectory_from_opencode_json(text: str) -> dict[str, Any] | None: "observation": {"results": []}, } observation_text = _opencode_tool_observation(state) - if observation_text: - step["observation"]["results"].append( - { - "source_call_id": call_id, - "content": observation_text, - } - ) + failed_read = status == "error" and function_name.lower() in {"read", "read_file"} + if observation_text or failed_read: + result = {"source_call_id": call_id, "content": observation_text} + if failed_read: + result["extra"] = {"tool_result_is_error": True} + step["observation"]["results"].append(result) steps.append(step) if not saw_content or not steps: diff --git a/src/skillevaluator/tier3/harbor/templates/eval.py b/src/skillevaluator/tier3/harbor/templates/eval.py index 7e1aeec0..0e0753eb 100644 --- a/src/skillevaluator/tier3/harbor/templates/eval.py +++ b/src/skillevaluator/tier3/harbor/templates/eval.py @@ -392,11 +392,15 @@ def get_all_tool_calls(traj): def get_skill_tool_calls(traj): skills = [] - for tc in get_all_tool_calls(traj): - if tc["fn"].lower() == "skill": - name = tc["args"].get("skill", tc["args"].get("name", "")) - if name: - skills.append(str(name)) + for step, tc in iter_tool_calls(traj): + if (tc.get("function_name") or "").lower() != "skill": + continue + if normalized_tool_call_is_error(step, tc): + continue + args = tc.get("arguments") or {} + name = args.get("skill", args.get("name", "")) + if name: + skills.append(str(name)) return skills @@ -3147,6 +3151,24 @@ def _command_reads_skill_md_arg(command, cmd_idx, assignments): return False +def _failed_single_skill_read(call): + """A shell failure only proves the skill read failed for an isolated read. + + Compound commands and multi-file reads may print SKILL.md successfully + before an unrelated operation fails. + """ + if call.get("is_error") is not True: + return False + tokens = _shell_tokens(_command_text(call)) + if not tokens or any(token in _SHELL_SEPARATORS for token in tokens): + return False + command = tokens[0].rsplit("/", 1)[-1].lower() + if command not in {"cat", "nl", "bat", "less", "more"}: + return False + files = [arg for arg in tokens[1:] if not arg.startswith("-")] + return len(files) == 1 and _skill_md_arg(files[0], {}) + + def _cmd_reads_skill_md(cmd) -> bool: """True if a shell command reads a SKILL.md via a file-view utility. @@ -4479,7 +4501,7 @@ def check_activation(tool_calls, expected_skill, skill_tool_names=None, acceptab "details": {**_skill_match_details(expected_skill, acceptable_skills), **match, "path": path_arg}, } for call in tool_calls: - if _is_execution_action(call["action"]) and call.get("is_error") is not True: + if _is_execution_action(call["action"]) and not _failed_single_skill_read(call): cmd = _command_text(call) if _cmd_reads_skill_md(cmd): match = _classify_skill_match(cmd, expected_skill, acceptable_skills, fuzzy=True) @@ -4511,7 +4533,10 @@ def check_activation(tool_calls, expected_skill, skill_tool_names=None, acceptab has_skill_read_evidence = ("read" in action.lower() and "SKILL.md" in str(tc.get("action_input", ""))) or ( _is_execution_action(action) and _cmd_reads_skill_md(cmd) ) - if not has_skill_read_evidence or tc.get("is_error") is True: + if not has_skill_read_evidence or ( + tc.get("is_error") is True + and (not _is_execution_action(action) or _failed_single_skill_read(tc)) + ): continue obs = str(tc.get("observation", "")).lower() if "skill.md" in obs: @@ -7528,7 +7553,9 @@ def check_workflow_order(tool_calls, skill_tool_names=None, expected_skill=None) args_str = str(call.get("action_input", "")) cmd = _command_text(call) if ("read" in action and "SKILL.md" in args_str) or (_is_execution_action(action) and _cmd_reads_skill_md(cmd)): - if call.get("is_error") is not True: + if call.get("is_error") is not True or ( + _is_execution_action(action) and not _failed_single_skill_read(call) + ): sequence.append("read_skill") elif _is_execution_action(action) and cmd and "--help" not in cmd and "which " not in cmd: sequence.append("execution") @@ -7640,7 +7667,7 @@ def check_routing( for call in tool_calls: action = call["action"].lower() cmd = _command_text(call) - if call.get("is_error") is True or not (_is_execution_action(action) and _cmd_reads_skill_md(cmd)): + if not (_is_execution_action(action) and _cmd_reads_skill_md(cmd)) or _failed_single_skill_read(call): continue skills_read.append(cmd) skill_name = _skill_name_from_ref(cmd) diff --git a/tests/tier3/test_failed_skill_reads.py b/tests/tier3/test_failed_skill_reads.py index 04a2e925..419d4eda 100644 --- a/tests/tier3/test_failed_skill_reads.py +++ b/tests/tier3/test_failed_skill_reads.py @@ -102,6 +102,58 @@ def test_failed_read_does_not_satisfy_workflow_before_execution( assert scorer.check_routing([read, retry], "demo-skill")["score"] == 1.0 +@pytest.mark.parametrize("command", ( + "cat /skills/demo-skill/SKILL.md && ls /skills/demo-skill/scripts/", + "cat /skills/demo-skill/SKILL.md /skills/demo-skill/reference.md", +)) +def test_failed_compound_shell_read_keeps_credit(grader, command) -> None: + _, scorer = grader + read = {"action": "Bash", "action_input": {"command": command}, + "observation": "# Demo skill", "is_error": True} + execute = {"action": "Bash", "action_input": {"command": "python scripts/run.py"}} + assert scorer.check_activation([read], "demo-skill")["score"] == 0.75 + assert scorer.check_workflow_order([read, execute], expected_skill="demo-skill")["score"] == 1.0 + assert scorer.check_routing([read], "demo-skill")["score"] == 1.0 + + +def test_failed_native_skill_call_is_not_activation(grader) -> None: + extractor, scorer = grader + trajectory = { + "steps": [{ + "source": "agent", "message": "", + "tool_calls": [{"tool_call_id": "s1", "function_name": "Skill", + "arguments": {"skill": "demo-skill"}}], + "observation": {"results": [{ + "source_call_id": "s1", "content": "Skill not found", + "extra": {"tool_result_is_error": True}, + }]}, + }] + } + calls = extractor.extract_tool_calls_as_dicts(trajectory) + assert extractor.get_skill_tool_calls(trajectory) == [] + assert scorer.check_activation(calls, "demo-skill", skill_tool_names=[])["score"] == 0.0 + assert scorer.check_workflow_order(calls, skill_tool_names=[], expected_skill="demo-skill")["score"] == 0.0 + assert scorer.check_routing(calls, "demo-skill", skill_tool_names=[])["score"] == 0.0 + + +def test_opencode_failed_read_is_marked_as_error(grader) -> None: + from skillevaluator.tier3.eval_core.log_converters import synthetic_trajectory_from_opencode_json + extractor, scorer = grader + event = {"type": "tool_use", "part": { + "tool": "read", "callID": "failed", "state": { + "status": "error", + "input": {"filePath": "/skills/demo-skill/SKILL.md"}, + "error": {"message": "File not found"}, + }, + }} + trajectory = synthetic_trajectory_from_opencode_json(json.dumps(event)) + assert trajectory is not None + calls = extractor.extract_tool_calls_as_dicts(trajectory) + assert calls[0]["is_error"] is True + assert scorer.check_activation(calls, "demo-skill")["score"] == 0.0 + assert scorer.check_routing(calls, "demo-skill")["score"] == 0.0 + + def test_native_atif_error_is_matched_to_its_own_call(grader: tuple[ModuleType, ModuleType]) -> None: from harbor.models.trajectories.trajectory import Trajectory From 943c655869b052ac2db6f0d6764cb7f93b837745 Mon Sep 17 00:00:00 2001 From: YMuskrat <101849520+YMuskrat@users.noreply.github.com> Date: Tue, 13 Oct 2026 13:51:57 +0100 Subject: [PATCH 6/8] fix(tier3): retain read credit for unrelated errors and extra operands Require matching tool result IDs before treating a read as failed. Respect shell -- delimiters and stdin operands when identifying isolated reads. Cover both host and verifier, including actual shell output and Claude log conversion. Validation: 571 focused tests passed; make lint and make build passed. Full-suite attempts stalled in this environment on both this branch and unchanged main; a full-suite pass is not claimed. Signed-off-by: YMuskrat <101849520+YMuskrat@users.noreply.github.com> --- CHANGELOG.md | 3 +- src/skillevaluator/tier3/eval_core/checks.py | 9 +- .../eval_core/codex_tool_call_normalizer.py | 6 +- .../tier3/harbor/templates/eval.py | 9 +- tests/tier3/test_failed_skill_reads.py | 172 +++++++++++++++--- 5 files changed, 168 insertions(+), 31 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 35f0540e..b8201be5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,7 +18,8 @@ All notable changes to SkillEvaluator are documented in this file. - Preserve explicit Claude and OpenCode read failure metadata in reconstructed ATIF trajectories. Failed isolated SKILL.md reads and failed native Skill calls lose credit; compound shell failures remain ambiguous and retain - prior credit in host and verifier grading. + prior credit in host and verifier grading. Unattributed errors do not cancel + successful reads, and shell reads count dash-prefixed filenames after `--`. - Preserved multiline skill descriptions when saving and loading Tier 2 embedding catalogs. Descriptions can contain line breaks and tabs, while catalog identifiers and other control characters remain strictly validated. diff --git a/src/skillevaluator/tier3/eval_core/checks.py b/src/skillevaluator/tier3/eval_core/checks.py index 7592edf4..e12db644 100644 --- a/src/skillevaluator/tier3/eval_core/checks.py +++ b/src/skillevaluator/tier3/eval_core/checks.py @@ -654,7 +654,14 @@ def _failed_single_skill_read(call): command = tokens[0].rsplit("/", 1)[-1].lower() if command not in {"cat", "nl", "bat", "less", "more"}: return False - files = [arg for arg in tokens[1:] if not arg.startswith("-")] + files = [] + options_ended = False + for arg in tokens[1:]: + arg = arg.removeprefix(_QUOTED_SYNTAX_MARK) + if not options_ended and arg == "--": + options_ended = True + elif options_ended or arg == "-" or not arg.startswith("-"): + files.append(arg) return len(files) == 1 and _skill_md_arg(files[0], {}) diff --git a/src/skillevaluator/tier3/eval_core/codex_tool_call_normalizer.py b/src/skillevaluator/tier3/eval_core/codex_tool_call_normalizer.py index 8fe5de88..c1c5344c 100644 --- a/src/skillevaluator/tier3/eval_core/codex_tool_call_normalizer.py +++ b/src/skillevaluator/tier3/eval_core/codex_tool_call_normalizer.py @@ -88,10 +88,8 @@ def normalized_tool_call_is_error(step: dict[str, Any], tool_call: dict[str, Any tool_call_id = tool_call.get("tool_call_id") for result in (step.get("observation") or {}).get("results") or []: source_call_id = result.get("source_call_id") - if not ( - (tool_call_id and source_call_id == tool_call_id) - or (not source_call_id and tool_call.get("_atif_raw_tool_index", 0) == 0) - ): + # Unscoped observations cannot prove that this particular call failed. + if not tool_call_id or source_call_id != tool_call_id: continue extra = result.get("extra") or {} if isinstance(extra, dict) and extra.get("tool_result_is_error") is True: diff --git a/src/skillevaluator/tier3/harbor/templates/eval.py b/src/skillevaluator/tier3/harbor/templates/eval.py index 0e0753eb..7f306fc4 100644 --- a/src/skillevaluator/tier3/harbor/templates/eval.py +++ b/src/skillevaluator/tier3/harbor/templates/eval.py @@ -3165,7 +3165,14 @@ def _failed_single_skill_read(call): command = tokens[0].rsplit("/", 1)[-1].lower() if command not in {"cat", "nl", "bat", "less", "more"}: return False - files = [arg for arg in tokens[1:] if not arg.startswith("-")] + files = [] + options_ended = False + for arg in tokens[1:]: + arg = arg.removeprefix(_QUOTED_SYNTAX_MARK) + if not options_ended and arg == "--": + options_ended = True + elif options_ended or arg == "-" or not arg.startswith("-"): + files.append(arg) return len(files) == 1 and _skill_md_arg(files[0], {}) diff --git a/tests/tier3/test_failed_skill_reads.py b/tests/tier3/test_failed_skill_reads.py index 419d4eda..df4fe143 100644 --- a/tests/tier3/test_failed_skill_reads.py +++ b/tests/tier3/test_failed_skill_reads.py @@ -6,6 +6,9 @@ from __future__ import annotations import json +import shlex +import subprocess +from pathlib import Path from types import ModuleType import pytest @@ -21,7 +24,7 @@ def grader(request: pytest.FixtureRequest, harbor_eval_template: ModuleType) -> return harbor_eval_template, harbor_eval_template -def _claude_read(*, is_error: bool, content: str) -> dict: +def _claude_tool_result(*, name: str, arguments: dict, is_error: bool, content: str) -> dict: events = [ { "type": "assistant", @@ -30,8 +33,8 @@ def _claude_read(*, is_error: bool, content: str) -> dict: { "type": "tool_use", "id": "read-1", - "name": "Read", - "input": {"file_path": "/skills/demo-skill/SKILL.md"}, + "name": name, + "input": arguments, } ] }, @@ -55,6 +58,15 @@ def _claude_read(*, is_error: bool, content: str) -> dict: return trajectory +def _claude_read(*, is_error: bool, content: str) -> dict: + return _claude_tool_result( + name="Read", + arguments={"file_path": "/skills/demo-skill/SKILL.md"}, + is_error=is_error, + content=content, + ) + + def test_claude_failed_read_cannot_receive_perfect_execution_credit(grader: tuple[ModuleType, ModuleType]) -> None: extractor, scorer = grader trajectory = _claude_read(is_error=True, content="Error: File does not exist: /skills/demo-skill/SKILL.md") @@ -102,14 +114,17 @@ def test_failed_read_does_not_satisfy_workflow_before_execution( assert scorer.check_routing([read, retry], "demo-skill")["score"] == 1.0 -@pytest.mark.parametrize("command", ( - "cat /skills/demo-skill/SKILL.md && ls /skills/demo-skill/scripts/", - "cat /skills/demo-skill/SKILL.md /skills/demo-skill/reference.md", -)) +@pytest.mark.parametrize( + "command", + ( + "cat /skills/demo-skill/SKILL.md && ls /skills/demo-skill/scripts/", + "cat /skills/demo-skill/SKILL.md /skills/demo-skill/reference.md", + "cat /skills/demo-skill/SKILL.md -", + ), +) def test_failed_compound_shell_read_keeps_credit(grader, command) -> None: _, scorer = grader - read = {"action": "Bash", "action_input": {"command": command}, - "observation": "# Demo skill", "is_error": True} + read = {"action": "Bash", "action_input": {"command": command}, "observation": "# Demo skill", "is_error": True} execute = {"action": "Bash", "action_input": {"command": "python scripts/run.py"}} assert scorer.check_activation([read], "demo-skill")["score"] == 0.75 assert scorer.check_workflow_order([read, execute], expected_skill="demo-skill")["score"] == 1.0 @@ -119,15 +134,22 @@ def test_failed_compound_shell_read_keeps_credit(grader, command) -> None: def test_failed_native_skill_call_is_not_activation(grader) -> None: extractor, scorer = grader trajectory = { - "steps": [{ - "source": "agent", "message": "", - "tool_calls": [{"tool_call_id": "s1", "function_name": "Skill", - "arguments": {"skill": "demo-skill"}}], - "observation": {"results": [{ - "source_call_id": "s1", "content": "Skill not found", - "extra": {"tool_result_is_error": True}, - }]}, - }] + "steps": [ + { + "source": "agent", + "message": "", + "tool_calls": [{"tool_call_id": "s1", "function_name": "Skill", "arguments": {"skill": "demo-skill"}}], + "observation": { + "results": [ + { + "source_call_id": "s1", + "content": "Skill not found", + "extra": {"tool_result_is_error": True}, + } + ] + }, + } + ] } calls = extractor.extract_tool_calls_as_dicts(trajectory) assert extractor.get_skill_tool_calls(trajectory) == [] @@ -138,14 +160,20 @@ def test_failed_native_skill_call_is_not_activation(grader) -> None: def test_opencode_failed_read_is_marked_as_error(grader) -> None: from skillevaluator.tier3.eval_core.log_converters import synthetic_trajectory_from_opencode_json + extractor, scorer = grader - event = {"type": "tool_use", "part": { - "tool": "read", "callID": "failed", "state": { - "status": "error", - "input": {"filePath": "/skills/demo-skill/SKILL.md"}, - "error": {"message": "File not found"}, + event = { + "type": "tool_use", + "part": { + "tool": "read", + "callID": "failed", + "state": { + "status": "error", + "input": {"filePath": "/skills/demo-skill/SKILL.md"}, + "error": {"message": "File not found"}, + }, }, - }} + } trajectory = synthetic_trajectory_from_opencode_json(json.dumps(event)) assert trajectory is not None calls = extractor.extract_tool_calls_as_dicts(trajectory) @@ -203,3 +231,99 @@ def test_native_atif_error_is_matched_to_its_own_call(grader: tuple[ModuleType, assert not calls[1].get("is_error") assert scorer.score_skill_execution(calls, "demo-skill")["score"] == 1.0 assert scorer.check_routing(calls, "demo-skill")["score"] == 1.0 + + +@pytest.mark.parametrize("action", ("Read", "Skill")) +def test_unrelated_error_does_not_cancel_successful_skill_read( + grader: tuple[ModuleType, ModuleType], action: str +) -> None: + from harbor.models.trajectories.trajectory import Trajectory + + extractor, scorer = grader + trajectory = Trajectory.model_validate( + { + "schema_version": "ATIF-v1.7", + "agent": {"name": "claude-code", "version": "test"}, + "steps": [ + { + "step_id": 1, + "source": "agent", + "message": "", + "tool_calls": [ + { + "tool_call_id": "read-1", + "function_name": action, + "arguments": {"file_path": "/skills/demo-skill/SKILL.md"} + if action == "Read" + else {"skill": "demo-skill"}, + } + ], + "observation": { + "results": [ + { + "source_call_id": "read-1", + "content": "# Demo skill", + "extra": {"tool_result_is_error": False}, + }, + { + "source_call_id": None, + "content": "Unrelated system action failed", + "extra": {"tool_result_is_error": True}, + }, + ] + }, + } + ], + } + ).to_json_dict() + calls = extractor.extract_tool_calls_as_dicts(trajectory) + names = extractor.get_skill_tool_calls(trajectory) + + assert not calls[0].get("is_error") + assert names == (["demo-skill"] if action == "Skill" else []) + assert scorer.score_skill_execution(calls, "demo-skill", skill_tool_names=names)["score"] == 1.0 + assert scorer.check_routing(calls, "demo-skill", skill_tool_names=names)["score"] == 1.0 + + +@pytest.mark.parametrize("delimiter", ("--", "'--'")) +def test_failed_cat_with_dash_prefixed_second_file_keeps_read_credit( + grader: tuple[ModuleType, ModuleType], tmp_path: Path, delimiter: str +) -> None: + extractor, scorer = grader + skill = tmp_path / "demo-skill" / "SKILL.md" + skill.parent.mkdir() + skill.write_text("# Demo skill\nActual instructions\n") + command = f"cat {delimiter} {shlex.quote(str(skill))} -missing-reference.md" + result = subprocess.run(["bash", "-c", command], cwd=tmp_path, capture_output=True, text=True) + assert result.returncode == 1 + assert result.stdout == skill.read_text() + assert "-missing-reference.md" in result.stderr + trajectory = _claude_tool_result( + name="Bash", + arguments={"command": command}, + is_error=True, + content=result.stdout + result.stderr, + ) + calls = extractor.extract_tool_calls_as_dicts(trajectory) + + assert calls[0]["is_error"] is True + assert scorer.check_activation(calls, "demo-skill")["score"] == 0.75 + assert scorer.check_workflow_order(calls, expected_skill="demo-skill")["score"] == 1.0 + assert scorer.check_routing(calls, "demo-skill")["score"] == 1.0 + + +@pytest.mark.parametrize("options", ("--", "'--'", "-n", "-n --")) +@pytest.mark.parametrize("is_error", (False, True)) +def test_single_cat_read_with_options(grader: tuple[ModuleType, ModuleType], options: str, is_error: bool) -> None: + extractor, scorer = grader + trajectory = _claude_tool_result( + name="Bash", + arguments={"command": f"cat {options} /skills/demo-skill/SKILL.md"}, + is_error=is_error, + content="File not found" if is_error else "# Demo skill", + ) + calls = extractor.extract_tool_calls_as_dicts(trajectory) + + assert scorer.check_activation(calls, "demo-skill")["score"] == (0.0 if is_error else 0.75) + assert scorer.check_workflow_order(calls, expected_skill="demo-skill")["score"] == (0.0 if is_error else 1.0) + assert scorer.check_routing(calls, "demo-skill")["score"] == (0.0 if is_error else 1.0) From 74819754e3bfd49fda8a5faf41982e5c2b91dff0 Mon Sep 17 00:00:00 2001 From: Narendran Raghavan Date: Tue, 13 Oct 2026 19:18:57 -0700 Subject: [PATCH 7/8] fix(tier3): keep read credit when a shell path can expand A single shell operand such as `cat skills/{demo-skill,missing}/SKILL.md` or `cat skills/*/SKILL.md` can expand into several files. The command prints SKILL.md and then exits 1 for the other file, but it was counted as one isolated read, so the failure removed activation, workflow and routing credit. A shell failure now cancels a read only when the operand has no brace, glob, `$` or backtick, so it names exactly one file. Expanding operands keep their previous credit. Tilde paths still count as one file, since tilde expansion always yields one path. Host checks and the Harbor verifier share the rule, and both are covered by a real brace-expansion run. Co-Authored-By: Claude Opus 5.5 Signed-off-by: Narendran Raghavan --- CHANGELOG.md | 8 ++- src/skillevaluator/tier3/eval_core/checks.py | 13 +++- .../tier3/harbor/templates/eval.py | 13 +++- tests/tier3/test_failed_skill_reads.py | 61 +++++++++++++++++++ 4 files changed, 86 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b8201be5..154b5e2c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,9 +17,11 @@ All notable changes to SkillEvaluator are documented in this file. - Preserve explicit Claude and OpenCode read failure metadata in reconstructed ATIF trajectories. Failed isolated SKILL.md reads and failed native Skill - calls lose credit; compound shell failures remain ambiguous and retain - prior credit in host and verifier grading. Unattributed errors do not cancel - successful reads, and shell reads count dash-prefixed filenames after `--`. + calls lose credit; compound shell failures and shell reads whose path may + expand to several files (braces, globs, `$` or backticks) remain ambiguous + and retain prior credit in host and verifier grading. Unattributed errors do + not cancel successful reads, and shell reads count dash-prefixed filenames + after `--`. - Preserved multiline skill descriptions when saving and loading Tier 2 embedding catalogs. Descriptions can contain line breaks and tabs, while catalog identifiers and other control characters remain strictly validated. diff --git a/src/skillevaluator/tier3/eval_core/checks.py b/src/skillevaluator/tier3/eval_core/checks.py index e12db644..aa21ec94 100644 --- a/src/skillevaluator/tier3/eval_core/checks.py +++ b/src/skillevaluator/tier3/eval_core/checks.py @@ -640,11 +640,18 @@ def _command_reads_skill_md_arg(command: list[str], cmd_idx: int, assignments: d return False +# Brace, glob, parameter and command expansion can turn one shell word into +# several files. Quotes are gone after tokenizing, so any operand holding one of +# these characters is treated as possibly naming more than one file. +_SHELL_EXPANSION_CHARS = frozenset("{*?[$`") + + def _failed_single_skill_read(call): """A shell failure only proves the skill read failed for an isolated read. - Compound commands and multi-file reads may print SKILL.md successfully - before an unrelated operation fails. + Compound commands, multi-file reads and operands that the shell may expand + into several files can print SKILL.md successfully before another file or + operation fails. """ if call.get("is_error") is not True: return False @@ -662,7 +669,7 @@ def _failed_single_skill_read(call): options_ended = True elif options_ended or arg == "-" or not arg.startswith("-"): files.append(arg) - return len(files) == 1 and _skill_md_arg(files[0], {}) + return len(files) == 1 and not _SHELL_EXPANSION_CHARS.intersection(files[0]) and _skill_md_arg(files[0], {}) def _cmd_reads_skill_md(cmd) -> bool: diff --git a/src/skillevaluator/tier3/harbor/templates/eval.py b/src/skillevaluator/tier3/harbor/templates/eval.py index 7f306fc4..409fc65e 100644 --- a/src/skillevaluator/tier3/harbor/templates/eval.py +++ b/src/skillevaluator/tier3/harbor/templates/eval.py @@ -3151,11 +3151,18 @@ def _command_reads_skill_md_arg(command, cmd_idx, assignments): return False +# Brace, glob, parameter and command expansion can turn one shell word into +# several files. Quotes are gone after tokenizing, so any operand holding one of +# these characters is treated as possibly naming more than one file. +_SHELL_EXPANSION_CHARS = frozenset("{*?[$`") + + def _failed_single_skill_read(call): """A shell failure only proves the skill read failed for an isolated read. - Compound commands and multi-file reads may print SKILL.md successfully - before an unrelated operation fails. + Compound commands, multi-file reads and operands that the shell may expand + into several files can print SKILL.md successfully before another file or + operation fails. """ if call.get("is_error") is not True: return False @@ -3173,7 +3180,7 @@ def _failed_single_skill_read(call): options_ended = True elif options_ended or arg == "-" or not arg.startswith("-"): files.append(arg) - return len(files) == 1 and _skill_md_arg(files[0], {}) + return len(files) == 1 and not _SHELL_EXPANSION_CHARS.intersection(files[0]) and _skill_md_arg(files[0], {}) def _cmd_reads_skill_md(cmd) -> bool: diff --git a/tests/tier3/test_failed_skill_reads.py b/tests/tier3/test_failed_skill_reads.py index df4fe143..e69ea0de 100644 --- a/tests/tier3/test_failed_skill_reads.py +++ b/tests/tier3/test_failed_skill_reads.py @@ -312,6 +312,67 @@ def test_failed_cat_with_dash_prefixed_second_file_keeps_read_credit( assert scorer.check_routing(calls, "demo-skill")["score"] == 1.0 +def test_failed_brace_expanded_cat_keeps_read_credit(grader: tuple[ModuleType, ModuleType], tmp_path: Path) -> None: + extractor, scorer = grader + skill = tmp_path / "skills" / "demo-skill" / "SKILL.md" + skill.parent.mkdir(parents=True) + skill.write_text("---\nname: demo-skill\n---\n# Demo skill\n", encoding="utf-8") + command = f"cat {shlex.quote(str(tmp_path / 'skills'))}/{{demo-skill,missing}}/SKILL.md" + result = subprocess.run(["bash", "-c", command], cwd=tmp_path, capture_output=True, text=True, encoding="utf-8") + assert result.returncode == 1 + assert result.stdout == skill.read_text(encoding="utf-8") + assert "missing/SKILL.md" in result.stderr + trajectory = _claude_tool_result( + name="Bash", + arguments={"command": command}, + is_error=True, + content=result.stdout + result.stderr, + ) + calls = extractor.extract_tool_calls_as_dicts(trajectory) + + assert calls[0]["is_error"] is True + assert scorer.check_activation(calls, "demo-skill")["score"] == 0.75 + assert scorer.check_workflow_order(calls, expected_skill="demo-skill")["score"] == 1.0 + assert scorer.check_routing(calls, "demo-skill")["score"] == 1.0 + + +@pytest.mark.parametrize( + "command", + ( + "cat /skills/*/SKILL.md", + "cat /skills/[a-z]*/SKILL.md", + "cat /skills/demo-skill?/SKILL.md", + "cat $SKILLS_DIR/SKILL.md", + "cat `pwd`/SKILL.md", + ), +) +def test_failed_cat_with_expanding_operand_keeps_read_credit(grader, command: str) -> None: + _, scorer = grader + read = { + "action": "Bash", + "action_input": {"command": command}, + "observation": "---\nname: demo-skill\n---\n# Demo skill\ncat: /skills/other/SKILL.md: Is a directory", + "is_error": True, + } + execute = {"action": "Bash", "action_input": {"command": "python scripts/run.py"}} + assert scorer.check_activation([read], "demo-skill")["score"] == 0.75 + assert scorer.check_workflow_order([read, execute], expected_skill="demo-skill")["score"] == 1.0 + assert scorer.check_routing([read], "demo-skill")["score"] == 1.0 + + +def test_failed_cat_of_home_relative_skill_loses_credit(grader) -> None: + # Tilde expansion always yields one path, so this is still an isolated read. + _, scorer = grader + read = { + "action": "Bash", + "action_input": {"command": "cat ~/.claude/skills/demo-skill/SKILL.md"}, + "observation": "cat: /root/.claude/skills/demo-skill/SKILL.md: No such file or directory", + "is_error": True, + } + assert scorer.check_activation([read], "demo-skill")["score"] == 0.0 + assert scorer.check_routing([read], "demo-skill")["score"] == 0.0 + + @pytest.mark.parametrize("options", ("--", "'--'", "-n", "-n --")) @pytest.mark.parametrize("is_error", (False, True)) def test_single_cat_read_with_options(grader: tuple[ModuleType, ModuleType], options: str, is_error: bool) -> None: From ef0395056d92693e1f0fca129d839857eecfe40b Mon Sep 17 00:00:00 2001 From: YMuskrat <101849520+YMuskrat@users.noreply.github.com> Date: Wed, 14 Oct 2026 15:40:00 +0100 Subject: [PATCH 8/8] style(tier3): tidy merged ATIF helper imports Keep the structured-content helpers from main alongside the failed-read error helper. The merged code preserves the shell-expansion fix from 7481975. Validation: 500 focused tests passed; make lint and make build passed. The full test attempt timed out after 90 seconds, so no full-suite pass is claimed. Signed-off-by: YMuskrat <101849520+YMuskrat@users.noreply.github.com> --- src/skillevaluator/tier3/eval_core/atif_helpers.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/skillevaluator/tier3/eval_core/atif_helpers.py b/src/skillevaluator/tier3/eval_core/atif_helpers.py index 84263e97..9b531780 100644 --- a/src/skillevaluator/tier3/eval_core/atif_helpers.py +++ b/src/skillevaluator/tier3/eval_core/atif_helpers.py @@ -17,7 +17,11 @@ from typing import Any from skillevaluator.evidence import evidence_ref_identity -from skillevaluator.tier3.eval_core.codex_tool_call_normalizer import atif_content_has_text, atif_content_text, normalized_tool_call_is_error +from skillevaluator.tier3.eval_core.codex_tool_call_normalizer import ( + atif_content_has_text, + atif_content_text, + normalized_tool_call_is_error, +) from skillevaluator.tier3.eval_core.codex_tool_call_normalizer import ( iter_normalized_tool_calls as iter_tool_calls, )