fix(skill): clean up evaluator docs + capture per-task grader failures

Three loose ends from the perf review on 2b94014:

1. Module docstring "Flow" section read sequentially despite steps 3-4
   now running concurrently via asyncio.gather. Rewrote to describe the
   concurrent dispatch + the EVAL_CONCURRENCY semaphore bound.

2. EVAL_CONCURRENCY comment claimed "~4x the sequential baseline" — but
   the semaphore is a sliding window across the combined trigger +
   coverage pool, so realistic speedup ranges from 30/8 (~3.75x) to 8x
   depending on per-call latency variance. Replaced the misleading
   claim with the actual semantics.

3. asyncio.gather had no return_exceptions=True, so a single grader
   raising RuntimeError (e.g. MaxTurnsExceeded wrap) aborted the whole
   eval and discarded the other ~29 successful gradings. Added per-task
   exception capture: failed prompts now go onto trigger_errors /
   coverage_errors on EvalResult, are surfaced as a [WARN] block in
   `openkb skill eval`, and are excluded from the rate denominators
   (we don't know what the verdict would have been). Added
   trigger_scored property and reworked passed / pass_rate to match.

Plus one regression test covering: one trigger grader and one coverage
grader both raise on different prompts; the other prompts grade
normally and end up in their expected slots. 464 tests passing.

Author: KylinMountain

openkb/cli.py

@@ -1790,13 +1790,17 @@ def skill_eval(ctx, name, save_flag, eval_set_path, count):
 
     click.echo(f"\nEval set: {result.total} prompts")
     click.echo(
-        f"Trigger accuracy: {result.passed}/{result.total} "
+        f"Trigger accuracy: {result.passed}/{result.trigger_scored} "
         f"({result.pass_rate * 100:.0f}%)  "
         f"— does the description fire on the right questions?"
     )
-    scored = result.trigger_questions - len(result.coverage_ambiguous)
+    coverage_scored = (
+        result.trigger_questions
+        - len(result.coverage_ambiguous)
+        - len(result.coverage_errors)
+    )
     click.echo(
-        f"Body coverage:    {result.coverage_passed}/{scored} "
+        f"Body coverage:    {result.coverage_passed}/{coverage_scored} "
         f"({result.coverage_rate * 100:.0f}%)  "
         f"— does SKILL.md actually support what the description promises?"
     )
@@ -1822,10 +1826,23 @@ def skill_eval(ctx, name, save_flag, eval_set_path, count):
             tail = f" — {amb.reason}" if amb.reason else ""
             click.echo(f"  - {amb.prompt.question}{tail}")
 
+    if result.trigger_errors or result.coverage_errors:
+        click.echo(
+            f"\n[WARN] {len(result.trigger_errors)} trigger and "
+            f"{len(result.coverage_errors)} coverage grader call(s) "
+            f"failed and are excluded from the scores above:"
+        )
+        for err in result.trigger_errors:
+            click.echo(f"  - trigger:  {err.prompt.question} — {err.reason}")
+        for err in result.coverage_errors:
+            click.echo(f"  - coverage: {err.prompt.question} — {err.reason}")
+
     if (
         not result.misses
         and not result.coverage_misses
         and not result.coverage_ambiguous
+        and not result.trigger_errors
+        and not result.coverage_errors
     ):
         click.echo("\nAll prompts graded correctly with full body support.")
 

openkb/skill/evaluator.py

@@ -18,11 +18,15 @@
   2. Generator LLM produces N should-trigger + N should-not prompts,
      using description AND body so prompts reflect what the skill
      actually claims to cover, not just description vibes.
-  3. Trigger grader sees ONLY the description and answers
-     trigger / no-trigger for each prompt — same as before.
-  4. Alignment grader sees the SKILL.md body+references and each
-     should-trigger prompt; answers "supported / unsupported" — does the
-     skill have the substance to handle this question?
+  3. Trigger grader (description-only, runs on every prompt) and
+     alignment grader (body + references, runs on should-trigger
+     prompts only) are dispatched concurrently via ``asyncio.gather``,
+     bounded by ``EVAL_CONCURRENCY``. Each grader is independent so
+     the two passes overlap completely.
+  4. Failed gradings (e.g. ``MaxTurnsExceeded``) are captured per task
+     via ``return_exceptions=True`` so a single failure doesn't discard
+     the other ~29 successful gradings; errored prompts are surfaced
+     separately and excluded from the rate denominators.
   5. Report both pass rates and the specific misses.
 
 Uses the same LiteLLM model the rest of the KB uses (config.yaml). No
@@ -48,8 +52,11 @@
 REFERENCES_PREVIEW_BYTES = 4000  # cap reference content fed to the eval LLM
 # Bound on concurrent grader LLM calls in run_eval. Without this the
 # default count=10 would fire ~30 simultaneous requests, which most
-# providers rate-limit. 8 is a conservative starting point — runs ~4x
-# the sequential baseline while staying well under typical RPM caps.
+# providers rate-limit. 8 is a conservative starting point — the
+# semaphore acts as a sliding window across the combined trigger + coverage
+# pool, so realistic speedup ranges from 30/8 (~3.75x) to 8x depending
+# on per-call latency variance. Bumping this up trades rate-limit risk
+# for wall-clock latency.
 EVAL_CONCURRENCY = 8
 
 
@@ -86,37 +93,59 @@ class EvalResult:
     # grader-malfunction doesn't silently inflate ``coverage_misses`` and
     # deflate ``coverage_rate``.
     coverage_ambiguous: list[CoverageMiss] = field(default_factory=list)
+    # Prompts whose grader raised (typically ``RuntimeError`` from a
+    # ``MaxTurnsExceeded`` wrap or a malformed-response failure inside
+    # the SDK). Captured per-task via ``return_exceptions=True`` in
+    # ``run_eval`` so one failure doesn't discard the other ~29
+    # gradings. Errors are excluded from rate denominators — we don't
+    # know what the verdict would have been.
+    trigger_errors: list[CoverageMiss] = field(default_factory=list)
+    coverage_errors: list[CoverageMiss] = field(default_factory=list)
 
     @property
     def total(self) -> int:
         return len(self.prompts)
 
     @property
     def passed(self) -> int:
-        return self.total - len(self.misses)
+        return self.trigger_scored - len(self.misses)
+
+    @property
+    def trigger_scored(self) -> int:
+        """Trigger prompts the grader returned a verdict on (no error)."""
+        return self.total - len(self.trigger_errors)
 
     @property
     def pass_rate(self) -> float:
-        return self.passed / self.total if self.total else 0.0
+        scored = self.trigger_scored
+        return self.passed / scored if scored else 0.0
 
     @property
     def trigger_questions(self) -> int:
         return sum(1 for p in self.prompts if p.expected == "trigger")
 
     @property
     def coverage_passed(self) -> int:
-        # Ambiguous outputs are excluded from both numerator and
-        # denominator — see ``coverage_rate``.
-        scored = self.trigger_questions - len(self.coverage_ambiguous)
+        # Ambiguous and errored outputs are excluded from both numerator
+        # and denominator — see ``coverage_rate``.
+        scored = (
+            self.trigger_questions
+            - len(self.coverage_ambiguous)
+            - len(self.coverage_errors)
+        )
         return scored - len(self.coverage_misses)
 
     @property
     def coverage_rate(self) -> float:
         # Score only the trigger prompts the grader gave a clear verdict
-        # on. A garbled run that flips half the outputs to ambiguous
-        # should narrow the denominator, not pretend half the body is
-        # hollow.
-        scored = self.trigger_questions - len(self.coverage_ambiguous)
+        # on. A garbled run that flips half the outputs to ambiguous or
+        # errors out should narrow the denominator, not pretend half the
+        # body is hollow.
+        scored = (
+            self.trigger_questions
+            - len(self.coverage_ambiguous)
+            - len(self.coverage_errors)
+        )
         return self.coverage_passed / scored if scored else 0.0
 
 
@@ -397,18 +426,32 @@ async def _coverage(p: EvalPrompt) -> tuple[
     coverage_prompts = [p for p in eval_set if p.expected == "trigger"]
     coverage_tasks = [_coverage(p) for p in coverage_prompts]
 
+    # return_exceptions=True so one failed grader doesn't discard the
+    # other ~29 successful gradings. Errored prompts are surfaced
+    # separately on the result and excluded from rate denominators.
     trigger_results, coverage_results = await asyncio.gather(
-        asyncio.gather(*trigger_tasks),
-        asyncio.gather(*coverage_tasks),
+        asyncio.gather(*trigger_tasks, return_exceptions=True),
+        asyncio.gather(*coverage_tasks, return_exceptions=True),
     )
 
     # Walk inputs in original order so `result.*` lists are deterministic
     # even though the gather() above completed out of order.
     for prompt, graded in zip(eval_set, trigger_results):
+        if isinstance(graded, BaseException):
+            result.trigger_errors.append(
+                CoverageMiss(prompt=prompt, reason=str(graded))
+            )
+            continue
         if graded != prompt.expected:
             result.misses.append(EvalMiss(prompt=prompt, graded=graded))
 
-    for prompt, (verdict, reason) in zip(coverage_prompts, coverage_results):
+    for prompt, outcome in zip(coverage_prompts, coverage_results):
+        if isinstance(outcome, BaseException):
+            result.coverage_errors.append(
+                CoverageMiss(prompt=prompt, reason=str(outcome))
+            )
+            continue
+        verdict, reason = outcome
         if verdict == "ambiguous":
             result.coverage_ambiguous.append(
                 CoverageMiss(prompt=prompt, reason=reason)

tests/test_skill_evaluator.py

@@ -315,6 +315,49 @@ async def mixed_coverage(content, question, *, model):
     assert result.coverage_rate == pytest.approx(0.5)
 
 
+@pytest.mark.asyncio
+async def test_run_eval_captures_grader_failures_without_aborting(tmp_path):
+    """A single grader failure must NOT abort the whole eval. The failed
+    prompt goes into trigger_errors/coverage_errors and is excluded from
+    rate denominators; the other prompts grade normally."""
+    skill_dir = _make_skill(tmp_path)
+    eval_set = _build_eval_set(2, 2)  # 2 trigger + 2 no-trigger
+
+    async def flaky_trigger(description, question, *, model):
+        if question == "trig 0":
+            raise RuntimeError("max-turn cap hit")
+        # Everything else: perfect grader
+        match = next(p for p in eval_set if p.question == question)
+        return match.expected
+
+    async def flaky_coverage(content, question, *, model):
+        if question == "trig 1":
+            raise RuntimeError("malformed grader output")
+        return "supported", ""
+
+    with patch("openkb.skill.evaluator.grade_one", side_effect=flaky_trigger), \
+         patch("openkb.skill.evaluator.grade_coverage", side_effect=flaky_coverage):
+        result = await run_eval(skill_dir, model="gpt-4o-mini", eval_set=eval_set)
+
+    # 4 prompts total; one trigger errored, one coverage errored.
+    assert result.total == 4
+    assert len(result.trigger_errors) == 1
+    assert result.trigger_errors[0].prompt.question == "trig 0"
+    assert "max-turn cap" in result.trigger_errors[0].reason
+    assert len(result.coverage_errors) == 1
+    assert result.coverage_errors[0].prompt.question == "trig 1"
+    assert "malformed" in result.coverage_errors[0].reason
+
+    # Trigger: 3 prompts scored (1 errored), all correct -> 3/3 = 100%
+    assert result.trigger_scored == 3
+    assert result.passed == 3
+    assert result.pass_rate == pytest.approx(1.0)
+
+    # Coverage: 2 trigger prompts, 1 errored, 1 supported -> 1/1 = 100%
+    assert result.coverage_passed == 1
+    assert result.coverage_rate == pytest.approx(1.0)
+
+
 # -------- save/load round-trip ------------------------------------------------