diff --git a/src/coder_eval/criteria/skill_triggered.py b/src/coder_eval/criteria/skill_triggered.py index 3d1aaf65..999d8ce5 100644 --- a/src/coder_eval/criteria/skill_triggered.py +++ b/src/coder_eval/criteria/skill_triggered.py @@ -44,16 +44,22 @@ def _engaged_skill_names(cmd: CommandTelemetry) -> set[str]: """All skill names engaged by ONE command, agent-agnostically (any-skill). - Generalizes ``_engaged_skill`` (which asks about one named skill) so a - live verdict can detect a *competing* skill engagement. Collects: - - - the Claude ``Skill`` tool call's ``skill`` parameter, namespace-stripped - via ``.split(":")[-1]`` (the SDK reports ``plugin:skill-name``); and - - every ``skills//`` or ``skills\\\\`` path segment appearing - in any string parameter (the Codex / file-read signal — repo layout or - the ``.agents/skills`` sandbox symlink). - - Returns the (possibly empty) set of engaged skill names for this command. + Detects both engagement signals so the criterion scores identically across + agents, and returns the (possibly empty) set of engaged skill names: + + - Claude: an explicit ``Skill`` tool call carries the skill in + ``parameters['skill']``, optionally namespaced (e.g. + ``plugin:uipath-agents``); the namespace is stripped via ``.split(":")[-1]``. + - Codex (and any non-Claude agent): no ``Skill`` tool exists, so a skill is + engaged by reading its files off disk via shell. Both the repo layout + (``.../skills//...``) and the sandbox symlink + (``.agents/skills//...``) contain the substring ``skills//``, + matched here in any string parameter (Bash ``parameters['command']`` or a + file-path parameter). The trailing separator required by ``_SKILL_PATH_RE`` + prevents prefix collisions (``uipath-agents`` vs ``uipath-agents-foo``). + + Returning the full set (rather than a single-skill yes/no) lets callers detect + a *competing* skill engagement. """ names: set[str] = set() if cmd.tool_name == "Skill": @@ -66,21 +72,31 @@ def _engaged_skill_names(cmd: CommandTelemetry) -> set[str]: return names -def _engaged_skill(cmd: CommandTelemetry, skill_name: str) -> bool: - """True when one command engaged ``skill_name`` — agent-agnostically. +def _all_engaged_skill_names(turn_records: list[TurnRecord]) -> set[str]: + """Union of every skill engaged anywhere in the trajectory (any-engagement). - Claude: an explicit ``Skill`` tool call carries the skill in - ``parameters['skill']`` (optionally namespaced, e.g. ``plugin:uipath-agents``). + Activation is scored on whether a skill was engaged *at all* during the run, + not on which skill was engaged first. This has two consequences that the + first-engagement policy could not express: - Codex (and any non-Claude agent): no ``Skill`` tool exists, so the skill is - engaged by reading its files off disk via shell. Both the repo layout - (``.../skills//...``) and the sandbox symlink - (``.agents/skills//...``) contain the substring - ``skills//``, which appears in the recorded command string - (Bash ``parameters['command']``) or a file-path parameter. The trailing - slash prevents prefix collisions (``uipath-agents`` vs ``uipath-agents-foo``). + - **Recall (the positive criterion).** A row that engages the wrong skill + before eventually engaging the expected one is still credited for the + expected skill — reading a ``SKILL.md`` to compare candidates is + exploration, not commitment, so an earlier wrong touch must not fail the + row. + - **Precision (the distractor/negative criteria).** An unrelated skill + engaged *anywhere* is counted against its own criterion, so a positive row + that also fires an off-target skill (and a negative row that fires any + target skill) is penalized on that skill's confusion cell. + + Order is irrelevant to a set union; the scan is left in ``sequence_number`` + order purely for determinism. """ - return skill_name in _engaged_skill_names(cmd) + names: set[str] = set() + for turn in turn_records: + for cmd in turn.commands: + names.update(_engaged_skill_names(cmd)) + return names @register_criterion @@ -94,8 +110,11 @@ class SkillTriggeredChecker(BaseCriterion[SkillTriggeredCriterion]): criterion_type = "skill_triggered" # Observable mid-run: a Skill tool call (or a skill file read) is a positive - # event in the live stream, so both polarities are decidable the moment the - # agent first engages ANY skill (the first-engagement policy in live_verdict). + # event in the live stream. The TYPE can decide either polarity — a positive + # criterion live-passes when its expected skill is engaged, a + # distractor/negative one live-fails when its (wrong) skill is engaged — but + # any single INSTANCE decides only one of the two; see + # ``live_decidable_polarities``. live_stop_polarities: ClassVar[frozenset[str]] = frozenset({"pass", "fail"}) def _check_impl( @@ -116,9 +135,13 @@ def _check_impl( error="turn_records not provided to checker", ) - triggered: bool = any( - _engaged_skill(cmd, criterion.skill_name) for turn in turn_records for cmd in turn.commands - ) + # Any-engagement policy (mirrors ``live_verdict``): the row is scored on + # whether this skill was engaged AT ALL, regardless of order. A positive + # criterion (skill_name == expected_skill) passes iff the expected skill + # was engaged somewhere in the run — a wrong skill engaged first does not + # fail it (recall). A distractor/negative criterion fails on ANY + # engagement of its skill (precision). + triggered: bool = criterion.skill_name in _all_engaged_skill_names(turn_records) expected_yes: bool = criterion.expected_skill == criterion.skill_name score = 1.0 if triggered == expected_yes else 0.0 observed = _YES if triggered else _NO @@ -139,31 +162,53 @@ def live_verdict( criterion: SkillTriggeredCriterion, turn_records: list[TurnRecord], ) -> LiveVerdict: - """First-engagement policy: the FIRST observed skill engagement decides. - - Activation measures which skill the agent selects *first*, so every - stacked ``skill_triggered`` criterion is decided simultaneously by the - first command that engages any skill: - - - before any engagement -> ``"undecided"``; - - on the first command engaging some skill: ``observed = (skill_name in - engaged)``, ``expected = (expected_skill == skill_name)`` -> - ``"pass"`` iff they match, else ``"fail"``. - - This covers the "wrong skill loads" case (a positive wrong signal) and - negative rows (``expected_skill == ""`` -> any engagement of the target - fails that criterion). It is a *policy* made consistent by stopping: the - trajectory is frozen at the deciding event, so the standard checker on the - truncated trajectory agrees with this verdict by construction. + """Any-engagement latch: decide the instant THIS skill is engaged. + + Mirrors ``_check_impl``'s any-engagement policy, latched monotonically + over the growing partial trajectory: + + - this skill not engaged yet -> ``"undecided"`` (the final outcome still + depends on the rest of the run — the expected skill may load later, or + a distractor may yet fire); + - this skill engaged -> ``expected_skill == skill_name`` decides: + ``"pass"`` for a positive criterion (the expected skill loaded), + ``"fail"`` for a distractor/negative one (a wrong skill loaded). + + Because engagement is monotonic (a skill, once engaged, stays engaged), a + latched verdict never flips, so it agrees with ``_check_impl`` on the + frozen trajectory by construction — whether or not the run stopped early. + A positive criterion can therefore only ever live-``pass`` and a + distractor/negative one only ever live-``fail``; their *absence* is never + decidable mid-run (see ``live_decidable_polarities``). This is the change + from first-engagement: a wrong skill engaged first no longer live-fails a + positive row — the run keeps going so the expected skill can still load. + """ + if criterion.skill_name not in _all_engaged_skill_names(turn_records): + return "undecided" + return "pass" if criterion.expected_skill == criterion.skill_name else "fail" + + @classmethod + def live_decidable_polarities(cls, criterion: SkillTriggeredCriterion) -> frozenset[str]: + """Per-instance narrowing under the any-engagement latch. + + Unlike the type-level capability (``live_stop_polarities`` = both), a + single instance decides exactly one polarity: + + - a **positive** criterion (``skill_name == expected_skill``) can only + live-``pass`` (the expected skill engaging is a decidable hit; its + absence is not knowable mid-run); + - a **distractor/negative** criterion (``skill_name != expected_skill``, + including the ``expected_skill == ""`` negatives) can only + live-``fail`` (a wrong skill engaging is a decidable miss; its absence + is not). + + ``validate_early_stop`` gates the requested ``stop_when`` on this set, so + arming a positive with ``fail`` / a distractor with ``pass`` — or either + with ``decided`` (which needs both) — is rejected at resolution rather + than silently degrading to a full run. """ expected_yes = criterion.expected_skill == criterion.skill_name - for turn in turn_records: - for cmd in turn.commands: - engaged = _engaged_skill_names(cmd) - if engaged: - observed_yes = criterion.skill_name in engaged - return "pass" if observed_yes == expected_yes else "fail" - return "undecided" + return frozenset({"pass"}) if expected_yes else frozenset({"fail"}) def aggregate( self, diff --git a/src/coder_eval/formatting.py b/src/coder_eval/formatting.py index 26c296ff..15685eeb 100644 --- a/src/coder_eval/formatting.py +++ b/src/coder_eval/formatting.py @@ -80,9 +80,12 @@ def format_messages( continue type_name = type(msg).__name__ - # StreamEvent is a known SDK type used for token-delta capture - # elsewhere; don't surface it as an "unhandled" warning here. - if type_name == "StreamEvent": + # Known, non-transcript SDK types: StreamEvent carries token deltas + # (captured elsewhere) and RateLimitEvent is an out-of-band throttling + # notice the SDK interleaves into the stream. Neither is transcript + # content, so skip both rather than surfacing an "unhandled" warning. + # Matched by name (not import) to stay robust across SDK versions. + if type_name in ("StreamEvent", "RateLimitEvent"): continue if type_name not in warned_unknown_types: warned_unknown_types.add(type_name) diff --git a/src/coder_eval/models/results.py b/src/coder_eval/models/results.py index 9a424914..ef3336e7 100644 --- a/src/coder_eval/models/results.py +++ b/src/coder_eval/models/results.py @@ -431,7 +431,12 @@ class EarlyStopInfo(BaseModel): description="SDK inner-turn count at the stop (watcher counts TurnStartEvents). NOT the " + "orchestrator iteration, which is always 1 in single-shot." ) - tool_call_index: int = Field(description="1-based index of the tool call that decided the stop.") + tool_call_index: int = Field( + description="1-based index of the tool call that decided the stop. NOTE: because the " + + "stop latches on the tool CALL (ToolStartEvent), this is the index of the deciding " + + "call INCLUDING that in-flight call — i.e. completed_tool_ends + 1 for a call-latched " + + "stop. Read it as 'which call decided', not as a count of fully-completed tool calls." + ) elapsed_seconds: float = Field(description="Wall-clock seconds from the first agent-start event to the stop.") turns_remaining_at_stop: int | None = Field( default=None, diff --git a/src/coder_eval/orchestration/early_stop.py b/src/coder_eval/orchestration/early_stop.py index 7891fb23..a1d76fb6 100644 --- a/src/coder_eval/orchestration/early_stop.py +++ b/src/coder_eval/orchestration/early_stop.py @@ -9,13 +9,21 @@ never a silent no-op. * ``EarlyStopWatcher`` — the runtime observer. A ``StreamCallback`` composed into the agent's event stream that maintains its own ``EventCollector``, - evaluates every armed criterion's ``live_verdict`` on each tool completion, - applies the stop rule, and exposes ``should_stop()`` (the cooperative - interrupt the agent polls) plus ``info`` (the ``EarlyStopInfo`` the + evaluates every armed criterion's ``live_verdict`` on each tool *call* (and on + its result), applies the stop rule, and exposes ``should_stop()`` (the + cooperative interrupt the agent polls) plus ``info`` (the ``EarlyStopInfo`` the orchestrator records). Fail-open: a raising ``live_verdict`` disarms the watcher and degrades to a full run — a verdict bug can never cause a *false* early stop. +Deciding on the tool *call* (``ToolStartEvent``), not the result, is what makes +the stop robust: for an observable criterion the verdict is fully determined by +the call's inputs (which skill / which command), so the watcher can latch the +instant the call is dispatched — before a cut-short turn (e.g. a timeout) can +strip the result and leave the call unresolved. The agent polls ``should_stop`` +immediately after dispatching each message, so a latch on the call breaks the +loop before the result message is ever pulled. + Live verdicts only *trigger* the stop; the authoritative scores always come from the standard ``check_all`` on the frozen trajectory after the cut. """ @@ -28,12 +36,19 @@ from coder_eval.models import EarlyStopInfo, EarlyStopReason from coder_eval.streaming.collector import EventCollector -from coder_eval.streaming.events import AgentStartEvent, StreamEvent, ToolEndEvent, ToolEndStatus, TurnStartEvent +from coder_eval.streaming.events import ( + AgentStartEvent, + StreamEvent, + ToolEndEvent, + ToolEndStatus, + ToolStartEvent, + TurnStartEvent, +) if TYPE_CHECKING: from coder_eval.criteria.base import BaseCriterion, LiveVerdict - from coder_eval.models import BaseSuccessCriterion, TaskDefinition + from coder_eval.models import BaseSuccessCriterion, CommandTelemetry, TaskDefinition # Armed pair the watcher holds: (criterion model, its checker). Lives in the # TYPE_CHECKING block (only annotations reference it, and those are lazy under @@ -210,25 +225,35 @@ def for_task(cls, task: TaskDefinition) -> EarlyStopWatcher: # --- StreamCallback -------------------------------------------------- # def on_event(self, event: StreamEvent) -> None: - """Forward the event to the internal collector; evaluate on tool completions. + """Forward the event to the internal collector; evaluate on each tool call. Short-circuits once the decision is latched (fired or disarmed). Counts - ``TurnStartEvent`` for ``sdk_turn_index`` and ``ToolEndEvent`` for the - 1-based ``tool_call_index``, and stamps the wall-clock origin at the - FIRST ``AgentStartEvent`` only (a retry's second AgentStart does not + ``TurnStartEvent`` for ``sdk_turn_index`` and each dispatched tool call + for the 1-based ``tool_call_index``, and stamps the wall-clock origin at + the FIRST ``AgentStartEvent`` only (a retry's second AgentStart does not reset it). - UNRESOLVED tool ends are ignored entirely (not collected, counted, or - evaluated). ``_ClaudeTurnState.finalize`` force-closes orphaned tools as - UNRESOLVED *after* the message loop has ended and the terminal status is - already chosen (COMPLETED / TIMEOUT / crash) — those are not live tool - activity and must not trip the stop rule, or a run that ran to completion - (or timed out / crashed) would latch a false early stop (e.g. an - unresolved ``Skill`` call counts as engagement because ``skill_triggered`` - ignores ``result_status``). A legitimate stop only ever fires on an - OBSERVED tool result during the loop, so dropping unresolved ends can - never suppress a real stop; it also keeps a crashed attempt's orphan tools - out of the retry-persistent partial trajectory. + The decision is evaluated on the tool *call* (``ToolStartEvent``): for an + observable criterion the verdict is fully determined by the call's inputs, + so latching here lets the agent's post-dispatch ``should_stop`` poll break + the loop before a cut-short turn can strip the result. The call is not in + the collector yet (it reduces commands from ``ToolEndEvent``), so it is + passed to ``_evaluate`` as the in-flight command, reported at + ``tool_call_index + 1`` (it has no ``ToolEndEvent`` to count yet). The + matching ``ToolEndEvent`` still evaluates, which covers a verdict that only + becomes decidable once the result is known and is a no-op once a call has + already latched the stop. ``tool_call_index`` is incremented on the + resolved ``ToolEndEvent`` so it stays a count of completed tool calls. + + UNRESOLVED tool ends are ignored entirely (not counted or evaluated). + ``_ClaudeTurnState.finalize`` force-closes orphaned tools as UNRESOLVED + *after* the message loop has ended and the terminal status is already + chosen (COMPLETED / TIMEOUT / crash) — those are not live tool activity + and must not trip the stop rule, or a run that ran to completion (or timed + out / crashed) without a real, in-loop decision would latch a false early + stop. A legitimate stop always fires on the in-loop call, so dropping + unresolved ends can never suppress a real stop; it also keeps a crashed + attempt's orphan tools out of the retry-persistent partial trajectory. """ if self._info is not None or self._disarmed: return @@ -237,13 +262,19 @@ def on_event(self, event: StreamEvent) -> None: self._started_monotonic = time.monotonic() elif isinstance(event, TurnStartEvent): self._sdk_turn_index += 1 + elif isinstance(event, ToolStartEvent): + # Decide on the call, evaluating with it appended as the in-flight + # command (it has no ToolEnd to count yet, so report it as +1). + self._evaluate(in_flight=event.tool) + return elif isinstance(event, ToolEndEvent): if event.status == ToolEndStatus.UNRESOLVED: return self._tool_call_index += 1 - self._collector.on_event(event) - if isinstance(event, ToolEndEvent): + self._collector.on_event(event) self._evaluate() + return + self._collector.on_event(event) def should_stop(self) -> bool: """The cooperative interrupt the agent polls after each dispatched message.""" @@ -261,8 +292,18 @@ def disarmed(self) -> bool: # --- Stop rule -------------------------------------------------- # - def _evaluate(self) -> None: - records = [self._collector.build_turn_record()] + def _evaluate(self, in_flight: CommandTelemetry | None = None) -> None: + record = self._collector.build_turn_record() + if in_flight is not None: + # The in-flight call has no ToolEnd yet, so the collector (which + # reduces commands from ToolEnd) has not captured it. Append it so its + # engagement is visible to the verdict; re-sort by sequence to keep the + # partial trajectory in emission order. + record.commands = sorted([*record.commands, in_flight], key=lambda c: c.sequence_number) + records = [record] + # An in-flight call has not been counted by a ToolEnd yet, so report it as + # the next (1-based) tool call. + tool_call_index = self._tool_call_index + (1 if in_flight is not None else 0) verdicts: list[LiveVerdict] = [] for criterion, checker in self._armed: try: @@ -283,7 +324,7 @@ def _evaluate(self) -> None: # whose stop_when permits fail decides the run. for (criterion, _checker), verdict in zip(self._armed, verdicts, strict=True): if verdict == "fail" and criterion.stop_when in ("fail", "decided"): - self._fire(EarlyStopReason.CRITERION_FAILED, criterion) + self._fire(EarlyStopReason.CRITERION_FAILED, criterion, tool_call_index=tool_call_index) return # Pass-stop: EVERY armed criterion live-passes AND each permits pass. @@ -296,13 +337,13 @@ def _evaluate(self) -> None: for (criterion, _checker), verdict, prev in zip(self._armed, verdicts, self._prev_verdicts, strict=True): if verdict != prev: deciding = criterion - self._fire(EarlyStopReason.CRITERION_PASSED, deciding) + self._fire(EarlyStopReason.CRITERION_PASSED, deciding, tool_call_index=tool_call_index) return # No stop this round — record the verdicts so the next round can detect flips. self._prev_verdicts = verdicts - def _fire(self, reason: EarlyStopReason, criterion: BaseSuccessCriterion) -> None: + def _fire(self, reason: EarlyStopReason, criterion: BaseSuccessCriterion, *, tool_call_index: int) -> None: elapsed = 0.0 if self._started_monotonic is not None: elapsed = max(time.monotonic() - self._started_monotonic, 0.0) @@ -313,7 +354,7 @@ def _fire(self, reason: EarlyStopReason, criterion: BaseSuccessCriterion) -> Non deciding_criterion_description=criterion.description, armed_criteria=[f"{c.type}: {c.description}" for c, _ in self._armed], sdk_turn_index=self._sdk_turn_index, - tool_call_index=self._tool_call_index, + tool_call_index=tool_call_index, elapsed_seconds=elapsed, turns_remaining_at_stop=turns_remaining, ) @@ -323,6 +364,6 @@ def _fire(self, reason: EarlyStopReason, criterion: BaseSuccessCriterion) -> Non reason.value, criterion.type, self._sdk_turn_index, - self._tool_call_index, + tool_call_index, elapsed, ) diff --git a/tests/test_agent.py b/tests/test_agent.py index dcf36b9b..2e4f7aaa 100644 --- a/tests/test_agent.py +++ b/tests/test_agent.py @@ -671,6 +671,18 @@ class BareMessage: assert "[ASSISTANT] Second response" in formatted assert formatted.count("[ASSISTANT]") == 2 + # Test 12: RateLimitEvent — an out-of-band throttling notice the SDK + # interleaves into the stream. It is not transcript content, so it is skipped + # (matched by class name) and does NOT surface as an "unhandled" tag/warning. + class RateLimitEvent: + pass + + formatted = agent._format_messages([RateLimitEvent()]) + assert formatted == "[No output]" + # It also does not crowd out real content when interleaved. + formatted = agent._format_messages([RateLimitEvent(), _make_assistant("still here")]) + assert formatted == "[ASSISTANT] still here" + def test_format_messages_system_message_subclasses_are_filtered(): """Regression: SystemMessage SUBCLASSES (TaskStartedMessage, etc.) must diff --git a/tests/test_early_stop.py b/tests/test_early_stop.py index 33f6a694..80998978 100644 --- a/tests/test_early_stop.py +++ b/tests/test_early_stop.py @@ -70,6 +70,7 @@ AgentStartEvent, ToolEndEvent, ToolEndStatus, + ToolStartEvent, TurnEndStatus, TurnStartEvent, ) @@ -192,6 +193,18 @@ def _tool_end(cmd: CommandTelemetry) -> ToolEndEvent: return ToolEndEvent(task_id="t", tool=cmd) +def _skill_start(skill: str, *, tool_id: str = "sk-1", sequence_number: int = 0) -> ToolStartEvent: + """A Skill ToolStart (the tool CALL) engaging ``skill`` — no result yet.""" + cmd = CommandTelemetry( + tool_name="Skill", + tool_id=tool_id, + timestamp=_TS, + parameters={"skill": skill}, + sequence_number=sequence_number, + ) + return ToolStartEvent(task_id="t", tool=cmd) + + def _skill_events(skill: str, *, tool_id: str = "sk-1") -> list[Any]: """AgentStart + TurnStart + a Skill ToolEnd engaging ``skill``.""" return [_agent_start(), _turn_start(), _tool_end(_skill_cmd(skill, tool_id=tool_id))] @@ -269,10 +282,19 @@ def test_pass_when_expected_skill_engaged(self) -> None: rec = [_turn(_cmd("Skill", {"skill": "plugin:date-teller"}))] assert self.checker.live_verdict(crit, rec) == "pass" - def test_fail_when_wrong_skill_engaged(self) -> None: - # Positive row expecting date-teller, but a different skill loads first. + def test_positive_undecided_when_only_wrong_skill_engaged(self) -> None: + # A positive row expecting date-teller, but a different skill loads. Under + # any-engagement the positive criterion does NOT fail — date-teller may + # still load later, so the verdict stays undecided (the run keeps going). crit = _skill_crit("date-teller", "date-teller") rec = [_turn(_cmd("Skill", {"skill": "other-skill"}))] + assert self.checker.live_verdict(crit, rec) == "undecided" + + def test_distractor_fails_when_its_skill_engaged(self) -> None: + # A distractor criterion (skill_name != expected_skill): engaging its + # (wrong) skill is a decidable precision miss -> fail. + crit = _skill_crit("weather-teller", "date-teller") + rec = [_turn(_cmd("Skill", {"skill": "weather-teller"}))] assert self.checker.live_verdict(crit, rec) == "fail" def test_negative_row_target_engaged_is_fail(self) -> None: @@ -281,20 +303,33 @@ def test_negative_row_target_engaged_is_fail(self) -> None: rec = [_turn(_cmd("Skill", {"skill": "date-teller"}))] assert self.checker.live_verdict(crit, rec) == "fail" - def test_negative_row_other_engaged_is_pass(self) -> None: + def test_negative_undecided_when_other_engaged(self) -> None: + # A negative criterion cannot live-pass: the absence of its skill is not + # knowable mid-run, so an unrelated engagement leaves it undecided. crit = _skill_crit("date-teller", "") rec = [_turn(_cmd("Skill", {"skill": "unrelated"}))] - assert self.checker.live_verdict(crit, rec) == "pass" + assert self.checker.live_verdict(crit, rec) == "undecided" - def test_first_engagement_decides(self) -> None: - # The wrong skill engages first, the expected one later — first wins. + def test_expected_skill_engaged_after_wrong_is_pass(self) -> None: + # Item 1: the wrong skill engages first, the expected one later — the + # positive criterion passes (any-engagement, order-independent). crit = _skill_crit("date-teller", "date-teller") rec = [_turn(_cmd("Skill", {"skill": "wrong"}), _cmd("Skill", {"skill": "date-teller"}))] - assert self.checker.live_verdict(crit, rec) == "fail" + assert self.checker.live_verdict(crit, rec) == "pass" def test_polarities_declared(self) -> None: assert SkillTriggeredChecker.live_stop_polarities == frozenset({"pass", "fail"}) + def test_decidable_narrows_per_instance(self) -> None: + # A positive instance decides only pass; a distractor/negative only fail. + assert SkillTriggeredChecker.live_decidable_polarities(_skill_crit("date-teller", "date-teller")) == frozenset( + {"pass"} + ) + assert SkillTriggeredChecker.live_decidable_polarities( + _skill_crit("weather-teller", "date-teller") + ) == frozenset({"fail"}) + assert SkillTriggeredChecker.live_decidable_polarities(_skill_crit("date-teller", "")) == frozenset({"fail"}) + # --------------------------------------------------------------------------- # # command_executed live verdict @@ -396,10 +431,18 @@ def test_unobservable_checker_is_undecided(self) -> None: assert checker.live_verdict(crit, [_turn()]) == "undecided" def test_base_decidable_defaults_to_class_polarities(self) -> None: - # The base hook returns the ClassVar verbatim: skill_triggered does not - # narrow per-instance, so its decidable set equals its class capability. - crit = _skill_crit("s", "s") - assert SkillTriggeredChecker.live_decidable_polarities(crit) == SkillTriggeredChecker.live_stop_polarities + # The base hook returns the ClassVar verbatim for a criterion that does + # NOT override it: file_exists (unobservable) reports its empty capability. + init_criteria(validate=False) + checker_cls = type(CriterionRegistry.get_checker("file_exists")()) + crit = FileExistsCriterion(type="file_exists", path="x.txt", description="x") + assert checker_cls.live_decidable_polarities(crit) == checker_cls.live_stop_polarities == frozenset() + + def test_skill_triggered_decidable_is_subset_of_class_polarities(self) -> None: + # skill_triggered DOES narrow per-instance; each instance set stays a + # subset of the class capability. + for crit in (_skill_crit("s", "s"), _skill_crit("s", "other"), _skill_crit("s", "")): + assert SkillTriggeredChecker.live_decidable_polarities(crit) <= SkillTriggeredChecker.live_stop_polarities # --------------------------------------------------------------------------- # @@ -419,9 +462,28 @@ def test_unarmed_with_stop_when_is_inert(self) -> None: validate_early_stop(task) # no raise def test_armed_happy_path_accepts(self) -> None: - task = _task(criteria=[_skill_crit("s", "s", stop_when="decided")], stop_early=True) + # A positive skill_triggered decides only "pass", so arm it with pass. + task = _task(criteria=[_skill_crit("s", "s", stop_when="pass")], stop_early=True) + validate_early_stop(task) # no raise + + def test_armed_distractor_fail_accepts(self) -> None: + # A distractor (skill_name != expected_skill) decides only "fail". + task = _task(criteria=[_skill_crit("wrong", "s", stop_when="fail")], stop_early=True) validate_early_stop(task) # no raise + def test_skill_triggered_positive_fail_arm_rejected(self) -> None: + # A positive criterion can never live-fail; arming it with fail is a dead arm. + task = _task(criteria=[_skill_crit("s", "s", stop_when="fail")], stop_early=True) + with pytest.raises(EarlyStopConfigError, match="cannot decide polarity"): + validate_early_stop(task) + + def test_skill_triggered_decided_arm_rejected(self) -> None: + # A single skill_triggered instance decides only one polarity, so + # stop_when=decided (which needs both) can never be honored. + task = _task(criteria=[_skill_crit("s", "s", stop_when="decided")], stop_early=True) + with pytest.raises(EarlyStopConfigError, match="cannot decide polarity"): + validate_early_stop(task) + def test_armed_command_executed_accepts(self) -> None: # A decidable fail arm: must-NOT-run (max_count set) can live-fail. task = _task(criteria=[_cmd_crit(stop_when="fail", min_count=0, max_count=0)], stop_early=True) @@ -499,10 +561,12 @@ def test_raise_order_simulation_before_agent(self) -> None: validate_early_stop(task) def test_stacked_activation_criteria_accept(self) -> None: - # Multiple armed skill_triggered criteria (the activation pattern). + # The activation pattern under the any-engagement latch: the positive (GT) + # criterion arms pass, a distractor arms fail. `decided` is invalid for + # either because a single instance decides only one polarity. crits = [ - _skill_crit("skill-a", "skill-a", stop_when="decided"), - _skill_crit("skill-b", "skill-a", stop_when="decided"), + _skill_crit("skill-a", "skill-a", stop_when="pass"), # positive -> pass + _skill_crit("skill-b", "skill-a", stop_when="fail"), # distractor -> fail ] task = _task(criteria=crits, stop_early=True) validate_early_stop(task) # no raise @@ -527,7 +591,7 @@ def test_stacked_activation_criteria_accept(self) -> None: description: date-teller activation skill_name: date-teller expected_skill: date-teller - stop_when: decided + stop_when: pass """ @@ -836,7 +900,7 @@ def test_evaluation_result_roundtrip_with_early_stop(self) -> None: def test_armed_criteria_passed_gates_armed_only(self) -> None: # Armed skill passes; advisory file_exists fails. armed gate -> True. criteria = [ - _skill_crit("date-teller", "date-teller", stop_when="decided"), + _skill_crit("date-teller", "date-teller", stop_when="pass"), FileExistsCriterion(path="x", description="x must exist"), ] result = _result(criteria_results=[_crit_result("skill_triggered", 1.0), _crit_result("file_exists", 0.0)]) @@ -846,7 +910,7 @@ def test_armed_criteria_passed_gates_armed_only(self) -> None: def test_armed_criteria_passed_fails_when_armed_fails(self) -> None: criteria = [ - _skill_crit("date-teller", "date-teller", stop_when="decided"), + _skill_crit("date-teller", "date-teller", stop_when="pass"), FileExistsCriterion(path="x", description="x must exist"), ] result = _result(criteria_results=[_crit_result("skill_triggered", 0.0), _crit_result("file_exists", 1.0)]) @@ -880,7 +944,7 @@ class TestEarlyStopWatcher: def test_for_task_arms_only_stop_criteria(self) -> None: watcher = _watcher( [ - _skill_crit("date-teller", "date-teller", stop_when="decided"), + _skill_crit("date-teller", "date-teller", stop_when="pass"), FileExistsCriterion(path="x", description="x must exist"), ] ) @@ -888,50 +952,59 @@ def test_for_task_arms_only_stop_criteria(self) -> None: assert len(watcher._armed) == 1 def test_undecided_before_engagement_no_stop(self) -> None: - watcher = _watcher([_skill_crit("date-teller", "date-teller", stop_when="decided")]) + watcher = _watcher([_skill_crit("date-teller", "date-teller", stop_when="pass")]) _feed(watcher, [_agent_start(), _turn_start()]) assert watcher.should_stop() is False assert watcher.info is None def test_pass_stop_fires_on_expected_skill(self) -> None: - watcher = _watcher([_skill_crit("date-teller", "date-teller", stop_when="decided")]) + watcher = _watcher([_skill_crit("date-teller", "date-teller", stop_when="pass")]) _feed(watcher, _skill_events("date-teller")) assert watcher.should_stop() is True assert watcher.info is not None assert watcher.info.reason == EarlyStopReason.CRITERION_PASSED - def test_fail_stop_fires_on_wrong_skill(self) -> None: - watcher = _watcher([_skill_crit("date-teller", "date-teller", stop_when="decided")]) + def test_fail_stop_fires_on_distractor_skill(self) -> None: + # A distractor criterion (its skill != the expected skill) fail-stops the + # instant its skill is engaged — the per-skill precision signal. + watcher = _watcher([_skill_crit("weather-teller", "date-teller", stop_when="fail")]) _feed(watcher, _skill_events("weather-teller")) assert watcher.should_stop() is True assert watcher.info is not None assert watcher.info.reason == EarlyStopReason.CRITERION_FAILED - def test_pass_polarity_does_not_fire_on_fail(self) -> None: - # stop_when="pass": a wrong-skill (live-fail) engagement must NOT stop. + def test_wrong_skill_does_not_stop_positive_row(self) -> None: + # Item 1: a positive row (armed pass) engaging the WRONG skill must NOT + # stop — the run keeps going so the expected skill can still load later. watcher = _watcher([_skill_crit("date-teller", "date-teller", stop_when="pass")]) _feed(watcher, _skill_events("weather-teller")) assert watcher.should_stop() is False + assert watcher.info is None - def test_stacked_pass_stop(self) -> None: - # Two armed skill criteria, both expecting date-teller; engaging it passes both. + def test_stacked_pass_stop_requires_all(self) -> None: + # Pass-stop needs EVERY armed criterion to live-pass. Two positives for + # different skills: engaging only the first does not stop; engaging the + # second (both now passed) fires the pass-stop. watcher = _watcher( [ - _skill_crit("date-teller", "date-teller", stop_when="decided"), - _skill_crit("weather-teller", "", stop_when="decided"), + _skill_crit("date-teller", "date-teller", stop_when="pass"), + _skill_crit("weather-teller", "weather-teller", stop_when="pass"), ] ) _feed(watcher, _skill_events("date-teller")) + assert watcher.should_stop() is False # only one of two has passed + _feed(watcher, [_tool_end(_skill_cmd("weather-teller", tool_id="w"))]) assert watcher.should_stop() is True assert watcher.info is not None assert watcher.info.reason == EarlyStopReason.CRITERION_PASSED def test_stacked_wrong_skill_fail_stop(self) -> None: - # Engaging weather-teller: date-teller row -> fail; the fail-stop fires first. + # A positive (armed pass) + a distractor (armed fail). Engaging the + # distractor's skill fires the fail-stop while the positive stays undecided. watcher = _watcher( [ - _skill_crit("date-teller", "date-teller", stop_when="decided"), - _skill_crit("weather-teller", "date-teller", stop_when="decided"), + _skill_crit("date-teller", "date-teller", stop_when="pass"), + _skill_crit("weather-teller", "date-teller", stop_when="fail"), ] ) _feed(watcher, _skill_events("weather-teller")) @@ -939,26 +1012,26 @@ def test_stacked_wrong_skill_fail_stop(self) -> None: assert watcher.info.reason == EarlyStopReason.CRITERION_FAILED def test_records_turn_and_tool_index(self) -> None: - watcher = _watcher([_skill_crit("date-teller", "date-teller", stop_when="decided")]) + watcher = _watcher([_skill_crit("date-teller", "date-teller", stop_when="pass")]) _feed(watcher, _skill_events("date-teller")) assert watcher.info is not None assert watcher.info.sdk_turn_index == 1 assert watcher.info.tool_call_index == 1 def test_turns_remaining_from_max_turns(self) -> None: - watcher = _watcher([_skill_crit("date-teller", "date-teller", stop_when="decided")], max_turns=15) + watcher = _watcher([_skill_crit("date-teller", "date-teller", stop_when="pass")], max_turns=15) _feed(watcher, _skill_events("date-teller")) assert watcher.info is not None assert watcher.info.turns_remaining_at_stop == 14 # 15 - sdk_turn_index(1) def test_turns_remaining_none_when_max_turns_unset(self) -> None: - watcher = _watcher([_skill_crit("date-teller", "date-teller", stop_when="decided")], max_turns=None) + watcher = _watcher([_skill_crit("date-teller", "date-teller", stop_when="pass")], max_turns=None) _feed(watcher, _skill_events("date-teller")) assert watcher.info is not None assert watcher.info.turns_remaining_at_stop is None def test_fail_open_on_raising_verdict(self) -> None: - watcher = _watcher([_skill_crit("date-teller", "date-teller", stop_when="decided")]) + watcher = _watcher([_skill_crit("date-teller", "date-teller", stop_when="pass")]) with patch.object(SkillTriggeredChecker, "live_verdict", side_effect=RuntimeError("boom")): _feed(watcher, _skill_events("date-teller")) # Fail-open: disarmed, no false stop, degrades to a full run. @@ -971,7 +1044,7 @@ def test_unresolved_tool_end_does_not_latch(self) -> None: # loop ends and the terminal status is chosen. Such an orphan Skill # engagement must NOT trip a stop, else a naturally-completed (or # timed-out / crashed) run gets recorded as early-stopped. - watcher = _watcher([_skill_crit("date-teller", "date-teller", stop_when="decided")]) + watcher = _watcher([_skill_crit("date-teller", "date-teller", stop_when="pass")]) _feed(watcher, [_agent_start(), _turn_start(), _unresolved_skill_end("date-teller")]) assert watcher.should_stop() is False assert watcher.info is None @@ -980,7 +1053,7 @@ def test_unresolved_tool_end_does_not_latch(self) -> None: def test_resolved_after_unresolved_still_decides(self) -> None: # An UNRESOLVED end is dropped, but a later RESOLVED engagement still fires # the stop (dropping orphans never suppresses a real, observed stop). - watcher = _watcher([_skill_crit("date-teller", "date-teller", stop_when="decided")]) + watcher = _watcher([_skill_crit("date-teller", "date-teller", stop_when="pass")]) _feed(watcher, [_agent_start(), _turn_start(), _unresolved_skill_end("date-teller")]) assert watcher.info is None _feed(watcher, [_tool_end(_skill_cmd("date-teller", tool_id="sk-real"))]) @@ -989,7 +1062,7 @@ def test_resolved_after_unresolved_still_decides(self) -> None: assert watcher.info.reason == EarlyStopReason.CRITERION_PASSED def test_decision_latched_after_fire(self) -> None: - watcher = _watcher([_skill_crit("date-teller", "date-teller", stop_when="decided")]) + watcher = _watcher([_skill_crit("date-teller", "date-teller", stop_when="pass")]) _feed(watcher, _skill_events("date-teller")) fired = watcher.info # A subsequent (wrong-skill) engagement must not overwrite the latched decision. @@ -998,6 +1071,83 @@ def test_decision_latched_after_fire(self) -> None: assert watcher.info is not None assert watcher.info.reason == EarlyStopReason.CRITERION_PASSED + def test_tool_call_fires_before_result(self) -> None: + # The decision latches on the tool CALL (ToolStartEvent): a Skill call + # whose result never arrives (a cut-short turn would strip it) still stops. + # No ToolEndEvent is ever fed. + watcher = _watcher([_skill_crit("date-teller", "date-teller", stop_when="pass")]) + _feed(watcher, [_agent_start(), _turn_start(), _skill_start("date-teller")]) + assert watcher.should_stop() is True + assert watcher.info is not None + assert watcher.info.reason == EarlyStopReason.CRITERION_PASSED + # The in-flight call reports as the 1st tool call even without a ToolEnd. + assert watcher.info.tool_call_index == 1 + + def test_tool_call_distractor_fail_fires(self) -> None: + # A distractor (armed fail) fail-stops on the tool CALL that engages its + # skill, before any result arrives. + watcher = _watcher([_skill_crit("weather-teller", "date-teller", stop_when="fail")]) + _feed(watcher, [_agent_start(), _turn_start(), _skill_start("weather-teller")]) + assert watcher.info is not None + assert watcher.info.reason == EarlyStopReason.CRITERION_FAILED + + def test_tool_call_latches_on_file_read_engagement(self) -> None: + # Off-Claude agents (antigravity/codex) engage a skill by READING its files + # (skills//...), not via a Skill tool call. The watcher must latch on + # that Read ToolStart — the file-path parameter carries the signal on the + # call itself, so early-stop fires off-Claude just as it does for Claude. + watcher = _watcher([_skill_crit("date-teller", "date-teller", stop_when="pass")]) + read = CommandTelemetry( + tool_name="Read", + tool_id="r1", + timestamp=_TS, + parameters={"file_path": "/repo/skills/date-teller/SKILL.md"}, + ) + _feed(watcher, [_agent_start(), _turn_start(), ToolStartEvent(task_id="t", tool=read)]) + assert watcher.should_stop() is True + assert watcher.info is not None + assert watcher.info.reason == EarlyStopReason.CRITERION_PASSED + + def test_tool_call_latches_before_unresolved_end(self) -> None: + # The call fires the stop in-loop; a later finalize() UNRESOLVED end for + # the SAME call is short-circuited (decision already latched) — no relabel, + # no double count. + watcher = _watcher([_skill_crit("date-teller", "date-teller", stop_when="pass")]) + _feed(watcher, [_agent_start(), _turn_start(), _skill_start("date-teller", tool_id="sk-1")]) + fired = watcher.info + _feed(watcher, [_unresolved_skill_end("date-teller", tool_id="sk-1")]) + assert watcher.info is fired + assert watcher.info is not None + assert watcher.info.tool_call_index == 1 + + def test_tool_call_index_counts_prior_resolved_calls(self) -> None: + # A prior resolved, non-deciding tool is counted at its ToolEnd; the + # deciding in-flight call is then reported as the next (2nd) call. + watcher = _watcher([_skill_crit("date-teller", "date-teller", stop_when="pass")]) + prior = _cmd("Bash", {"command": "ls"}) # not a skill engagement + _feed(watcher, [_agent_start(), _turn_start(), _tool_end(prior)]) + assert watcher.info is None + _feed(watcher, [_skill_start("date-teller", tool_id="sk-1", sequence_number=1)]) + assert watcher.info is not None + assert watcher.info.tool_call_index == 2 + + def test_second_agent_start_does_not_reset_origin(self) -> None: + # The wall-clock origin is stamped at the FIRST AgentStartEvent only; a + # retry's second AgentStart must NOT reset it (the documented no-op branch + # in on_event). Exercised deterministically via _started_monotonic rather + # than the time-based elapsed_seconds field. + watcher = _watcher([_skill_crit("date-teller", "date-teller", stop_when="pass")]) + _feed(watcher, [_agent_start()]) + origin = watcher._started_monotonic + assert origin is not None + # A second AgentStart (as on a retry) must leave the origin untouched. + _feed(watcher, [_agent_start(), _turn_start()]) + assert watcher._started_monotonic == origin + # The stop that follows anchors elapsed_seconds to that first origin. + _feed(watcher, [_skill_start("date-teller")]) + assert watcher.info is not None + assert watcher.info.elapsed_seconds >= 0.0 + # --------------------------------------------------------------------------- # # Phase 3: Orchestrator wiring @@ -1088,13 +1238,20 @@ async def _run_wiring( class TestOrchestratorEarlyStopWiring: _SKILL = "date-teller" - def _criteria(self, *, expected: str = "date-teller", stop_when: str | None = "decided") -> list[Any]: - # Armed skill_triggered + advisory file_exists (deliberately failing). + def _criteria(self, *, expected: str = "date-teller", stop_when: str | None = "pass") -> list[Any]: + # Armed positive skill_triggered + advisory file_exists (deliberately failing). return [ _skill_crit(self._SKILL, expected, stop_when=stop_when), FileExistsCriterion(path="artifact.txt", description="artifact must exist"), ] + def _distractor_criteria(self) -> list[Any]: + # A distractor (armed fail) + advisory file_exists, for the fail-stop path. + return [ + _skill_crit("weather-teller", self._SKILL, stop_when="fail"), + FileExistsCriterion(path="artifact.txt", description="artifact must exist"), + ] + async def test_default_off_full_gate_no_early_stop(self, tmp_path) -> None: # Unarmed: no watcher, all criteria gate, advisory 0.0 drags to FAILURE. result, agent = await _run_wiring( @@ -1122,8 +1279,9 @@ async def test_pass_stop_cuts_the_stream(self, tmp_path) -> None: assert result.early_stop.reason == EarlyStopReason.CRITERION_PASSED async def test_fail_stop_wiring(self, tmp_path) -> None: + # A distractor (armed fail) fires the fail-stop when its skill is engaged. result, _agent = await _run_wiring( - criteria=self._criteria(), + criteria=self._distractor_criteria(), events=_skill_events("weather-teller"), scores=[0.0, 0.0], stop_early=True, @@ -1189,6 +1347,23 @@ async def test_completed_run_with_orphan_tool_not_early_stopped(self, tmp_path) assert agent.delivered == 3 # never stopped: the full stream was consumed assert result.all_criteria_passed(self._criteria()) is False + async def test_tool_call_cut_without_tool_end(self, tmp_path) -> None: + # End-to-end: the deciding Skill CALL (a ToolStart with no ToolEnd) cuts + # the stream and records an early stop — the case that would otherwise run + # to the turn cap when a cut-short turn strips the result. + events = [_agent_start(), _turn_start(), _skill_start(self._SKILL), _turn_start()] + result, agent = await _run_wiring( + criteria=self._criteria(), + events=events, + scores=[1.0, 0.0], + stop_early=True, + tmp_path=tmp_path, + ) + assert result.early_stop is not None + assert result.early_stop.reason == EarlyStopReason.CRITERION_PASSED + # Cut at the ToolStart: the trailing turn_start is never delivered. + assert agent.delivered == 3 + async def test_fail_open_wiring_degrades_to_full_run(self, tmp_path) -> None: with patch.object(SkillTriggeredChecker, "live_verdict", side_effect=RuntimeError("boom")): result, _agent = await _run_wiring( diff --git a/tests/test_skill_triggered.py b/tests/test_skill_triggered.py index ea1a8168..a856cde5 100644 --- a/tests/test_skill_triggered.py +++ b/tests/test_skill_triggered.py @@ -98,6 +98,215 @@ def test_no_turn_records_returns_base_result_with_error(self) -> None: assert result.error is not None +class TestSkillTriggeredAnyEngagement: + """Any-engagement scoring: a skill counts if it was engaged *at all*, in any + order. The expected skill passes its criterion (recall) even when a wrong + skill was touched first; an off-target skill still fails its own criterion + (precision). + """ + + def _admin_platform(self) -> list[CommandTelemetry]: + # The agent engages uipath-admin FIRST, then uipath-platform. + return [ + _cmd("Skill", {"skill": "uipath-admin"}, tool_id="s1"), + _cmd("Skill", {"skill": "uipath-platform"}, tool_id="s2"), + ] + + def test_expected_skill_engaged_first_is_true_positive(self) -> None: + # GT=uipath-admin; admin engaged (first) -> observed=yes, expected=yes. + result = _check(expected_skill="uipath-admin", skill_name="uipath-admin", commands=self._admin_platform()) + assert result.observed_label == "yes" and result.score == 1.0 + + def test_off_target_skill_engaged_is_false_positive(self) -> None: + # Same run scored for the uipath-platform criterion: platform WAS engaged + # (second), so on an admin row it is a precision miss -> observed=yes, + # expected=no -> score 0.0. This is the per-skill precision signal. + result = _check(expected_skill="uipath-admin", skill_name="uipath-platform", commands=self._admin_platform()) + assert result.observed_label == "yes" and result.score == 0.0 + + def test_expected_skill_engaged_after_wrong_still_passes(self) -> None: + # Item 1: the WRONG skill engages first, the expected one later. The GT + # criterion must still PASS — an earlier wrong touch (comparison, not + # commitment) does not fail the row. + commands = [ + _cmd("Skill", {"skill": "uipath-platform"}, tool_id="s1"), + _cmd("Skill", {"skill": "uipath-admin"}, tool_id="s2"), + ] + result = _check(expected_skill="uipath-admin", skill_name="uipath-admin", commands=commands) + assert result.observed_label == "yes" and result.score == 1.0 + + def test_negative_row_fails_on_any_engagement(self) -> None: + # Negative row (expected_skill == ""): engaging the target skill at all — + # even after an unrelated one — is a false positive. + commands = [ + _cmd("Skill", {"skill": "uipath-platform"}, tool_id="s1"), + _cmd("Skill", {"skill": "uipath-admin"}, tool_id="s2"), + ] + result = _check(expected_skill="", skill_name="uipath-admin", commands=commands) + assert result.observed_label == "yes" and result.score == 0.0 + + def test_stacked_recall_and_precision_on_one_trajectory(self) -> None: + # How the stacked criteria score a single positive row (GT=uipath-admin) + # on which the agent engaged BOTH the expected skill and an off-target one. + # The GT criterion credits recall (pass); the off-target criterion records + # a precision miss (fail). No precision hole: an extra engagement is never + # silently absorbed — it lands on its own skill's confusion cell. + commands = self._admin_platform() + recall = _check(expected_skill="uipath-admin", skill_name="uipath-admin", commands=commands) + precision = _check(expected_skill="uipath-admin", skill_name="uipath-platform", commands=commands) + assert recall.observed_label == "yes" and recall.score == 1.0 # recall: GT engaged + assert precision.observed_label == "yes" and precision.score == 0.0 # precision: off-target engaged + + +def _check_multi( + *, expected_skill: str, skill_name: str, turns: list[list[CommandTelemetry]] +) -> ClassificationCriterionResult: + criterion = SkillTriggeredCriterion( + description="did agent invoke a skill?", + expected_skill=expected_skill, + skill_name=skill_name, + ) + checker = SkillTriggeredChecker() + turn_records = [_turn(cmds) for cmds in turns] + result = checker.check(criterion, sandbox=None, turn_records=turn_records) # type: ignore[arg-type] + assert isinstance(result, ClassificationCriterionResult) + return result + + +class TestSkillTriggeredGoldenCorpus: + """Regression lock: pins observed_label AND score for the canonical + multi-skill trajectories through ``_check_impl``. Any future change to the + engagement policy (any- vs first-engagement) breaks these, forcing an + explicit acknowledgement of the methodology break — and a re-score/backfill + of historical activation P/R/F1 — before merge. + """ + + @pytest.mark.parametrize( + ("case", "expected_skill", "skill_name", "commands", "exp_observed", "exp_score"), + [ + ( + "single-target-recall", + "uipath-admin", + "uipath-admin", + [_cmd("Skill", {"skill": "uipath-admin"}, tool_id="s1")], + "yes", + 1.0, + ), + ( + "target-first-then-competitor-recall", + "uipath-admin", + "uipath-admin", + [ + _cmd("Skill", {"skill": "uipath-admin"}, tool_id="s1"), + _cmd("Skill", {"skill": "uipath-platform"}, tool_id="s2"), + ], + "yes", + 1.0, + ), + ( + "target-first-then-competitor-precision", + "uipath-admin", + "uipath-platform", + [ + _cmd("Skill", {"skill": "uipath-admin"}, tool_id="s1"), + _cmd("Skill", {"skill": "uipath-platform"}, tool_id="s2"), + ], + "yes", + 0.0, + ), + ( + "wrong-first-then-target-recall", + "uipath-admin", + "uipath-admin", + [ + _cmd("Skill", {"skill": "uipath-platform"}, tool_id="s1"), + _cmd("Skill", {"skill": "uipath-admin"}, tool_id="s2"), + ], + "yes", + 1.0, + ), + ( + "negative-target-engaged-false-positive", + "", + "uipath-admin", + [_cmd("Skill", {"skill": "uipath-admin"}, tool_id="s1")], + "yes", + 0.0, + ), + ( + "negative-no-engagement-true-negative", + "", + "uipath-admin", + [_cmd("Read", {"file_path": "notes.txt"})], + "no", + 1.0, + ), + ( + "file-read-target-recall", + "uipath-admin", + "uipath-admin", + [_cmd("Read", {"file_path": "skills/uipath-admin/SKILL.md"})], + "yes", + 1.0, + ), + ], + ) + def test_golden_corpus_scores_are_pinned( + self, + case: str, + expected_skill: str, + skill_name: str, + commands: list[CommandTelemetry], + exp_observed: str, + exp_score: float, + ) -> None: + result = _check(expected_skill=expected_skill, skill_name=skill_name, commands=commands) + assert result.observed_label == exp_observed, case + assert result.score == exp_score, case + + +class TestSkillTriggeredMultiTurnParity: + """Any-engagement holds across TurnRecords and for the file-read signal, so + the agent-agnostic parity CLAUDE.md stresses is asserted for the new branch. + """ + + def test_expected_skill_in_later_turn_still_recalls(self) -> None: + # Distractor engaged in turn 1, expected skill in turn 2. Recall must + # credit the GT criterion regardless of which turn the engagement lives in. + result = _check_multi( + expected_skill="uipath-admin", + skill_name="uipath-admin", + turns=[ + [_cmd("Skill", {"skill": "uipath-platform"}, tool_id="s1")], + [_cmd("Skill", {"skill": "uipath-admin"}, tool_id="s2")], + ], + ) + assert result.observed_label == "yes" and result.score == 1.0 + + def test_off_target_in_earlier_turn_is_precision_miss(self) -> None: + # The competitor engaged in an EARLIER turn than the GT skill still lands + # as a precision miss on its own criterion (no turn-order dependence). + turns = [ + [_cmd("Skill", {"skill": "uipath-platform"}, tool_id="s1")], + [_cmd("Skill", {"skill": "uipath-admin"}, tool_id="s2")], + ] + precision = _check_multi(expected_skill="uipath-admin", skill_name="uipath-platform", turns=turns) + assert precision.observed_label == "yes" and precision.score == 0.0 + + def test_file_read_parity_across_turns(self) -> None: + # Off-Claude parity: the agent READS skill files across turns (platform's + # in turn 1, admin's in turn 2). Recall/precision score identically to the + # Skill-tool path, order- and turn-independent. + turns = [ + [_cmd("Read", {"file_path": "skills/uipath-platform/SKILL.md"})], + [_cmd("Read", {"file_path": "skills/uipath-admin/reference.md"})], + ] + recall = _check_multi(expected_skill="uipath-admin", skill_name="uipath-admin", turns=turns) + precision = _check_multi(expected_skill="uipath-admin", skill_name="uipath-platform", turns=turns) + assert recall.observed_label == "yes" and recall.score == 1.0 + assert precision.observed_label == "yes" and precision.score == 0.0 + + class TestSkillTriggeredCodex: """Codex has no ``Skill`` tool — it engages a skill by reading its files via shell.