fix(skill-creator): preserve recorded token counts in benchmarks (#7033)

* fix(skill-creator): preserve recorded token counts in benchmarks

Read timing.json token measurements independently of grading duration and retain recorded zero values. Keep the existing character-count fallback for missing measurements and add benchmark/report regression coverage.

* fix(skillscan): distinguish token metrics from credentials

Recognize typed token counts, numeric benchmark summaries, and signed deltas so skill-creator documentation passes deterministic review. Preserve credential names, string values, duplicate-key detection, and cloud-token findings; add regression coverage and document the boundary.

---------

Co-authored-by: Willem Jiang <willem.jiang@gmail.com>

README.md

@@ -1465,6 +1465,10 @@ Discovery follows operator-managed directory symlinks, but skips links back to a
 
 Skill Markdown and bundled text resources use UTF-8. Skill-creator CLI and review utilities read and write text explicitly as UTF-8 so localized skills behave consistently across operating systems.
 
+Skill-creator benchmark aggregation uses recorded `timing.json` token counts even
+when `grading.json` already contains the run duration. Recorded zero counts are
+preserved; output character counts remain a legacy proxy only when token data is missing.
+
 `SKILL.md` YAML frontmatter must open and close with `---` on its own line (trailing spaces or tabs are allowed). A line such as `---suffix` is not a closing delimiter.
 
 Custom skill history preserves Unicode line separators inside saved content and metadata, keeping those revisions readable for history inspection and rollback. Malformed JSON history records are still rejected.
@@ -1626,6 +1630,8 @@ SkillScan treats HTTP/HTTPS scheme spellings and HTTP hostnames case-insensitive
 loopback (`[::1]`) URLs as local. External IPv6 endpoints still trigger network findings,
 and cloud-metadata hostname detection is case-insensitive.
 
+SkillScan recognizes unquoted integer model-usage counters such as `total_tokens` and `input_tokens` in skill examples as measurements. Benchmark token statistics and signed deltas are also recognized; credential fields and other string values remain subject to secret detection.
+
 For Python credential mappings, SkillScan checks literal values while treating ordinary dictionary keys as labels. A mapping such as `tokens = {"access_token": os.getenv("ACCESS_TOKEN")}` does not report a hardcoded credential. Keys matching a recognized cloud or API token format are still checked as embedded credentials.
 
 SkillScan blocks embedded legacy and modern `sk-` API tokens, including OpenAI project, service-account, and admin keys and Anthropic keys, even in Bearer headers without a secret assignment. Token bodies may contain `-` and `_`; finding evidence is fully redacted. Placeholder examples do not suppress detection of later credentials in the same file. Scheduled-task notification summaries also redact these key formats before sending them to external IM platforms.

backend/packages/harness/deerflow/skills/AGENTS.md

@@ -6,6 +6,8 @@
 - **Diagnostic scope**: only `yaml.YAMLError` produces a management diagnostic. Missing frontmatter, non-mapping or invalid metadata, non-UTF-8 files and read errors (including permission denied) remain excluded; the UI and README state those limits. An empty diagnostic list is not a package-validity check. `tests/test_skill_load_diagnostics.py` pins these exclusions and permission-error redaction without changing runtime parsing.
 - **History JSONL**: split records on LF, not Unicode text separators; preserve U+0085/U+2028/U+2029 in saved fields. Keep malformed JSON rejected. Tests: `tests/test_skill_history_jsonl.py`.
 
+- **SkillScan usage counters**: the text assignment sweep recognizes unquoted nonnegative integers under the exact model-usage names `tokens`, `total_tokens`, `input_tokens`, `output_tokens`, `prompt_tokens`, `completion_tokens`, `cached_tokens`, and `reasoning_tokens`. Benchmark `tokens` also accepts a signed numeric delta or an object containing exactly four nonnegative finite numeric statistics (`mean`, `stddev`, `min`, `max`); JSON parsing is capped at 512 characters. Recover the complete key before applying the exception. Quoted unsigned counts, credential prefixes, arbitrary containers, and cloud-token detection remain active. Regressions live in `tests/test_skillscan_secret_assignment_formats.py`.
+
 - **SkillScan API tokens**: `secret-cloud-token` accepts `-` and `_` in the `sk-` family, matching the character class used by PII redaction. Legacy, OpenAI project/service-account/admin, and Anthropic keys must trigger `CRITICAL` even in a Bearer header without a secret assignment. Skip placeholder matches until a real credential is found, retaining at most one cloud-token finding per file. Keep evidence fully redacted and preserve the minimum length and word boundaries; synthetic regressions live in `tests/test_skillscan_native.py`. Keep the mirrored `sk-` pattern in `app/scheduler/notification_text.py` aligned so scheduled-run summaries scrub the same key formats before IM delivery.
 
 - **SkillScan shell pipes**: resolve `env` launchers through interpreter paths and `command`/`exec`/`sudo` wrappers, including value-taking sudo flags. Preserve direct-pipe coverage of common shell interpreter paths/names and real backslash-newline continuations; keep repeated regex alternatives disjoint so failed matches on literal backslash text cannot stall scanning. Tokenize only the current simple command, in place, under a shared per-file work budget; only its stdin redirections override the pipe. Keep quoted words distinct from operators. Parse GNU `env -S` escapes separately from shell syntax, with iterative expansion capped at 256 splits/65,536 characters. Unknown runtime expansions remain unresolved; never read the scanner environment. Limit exhaustion retains warnings and already collected critical findings. Tests: `test_skillscan_env_review_regressions.py`, `test_skillscan_env_launcher.py`, and the existing native/shell suites.

backend/packages/harness/deerflow/skills/skillscan/orchestrator.py

@@ -12,7 +12,9 @@ from __future__ import annotations
 
 import ast
 import io
+import json
 import logging
+import math
 import posixpath
 import re
 import stat
@@ -126,6 +128,8 @@ _PLACEHOLDER_VALUES = {"", "x", "xx", "xxx", "xxxx", "changeme", "change-me", "e
 # word (`tokenizer`, `secretive`) still stays quiet, so this is not a blanket match.
 _SECRET_ASSIGNMENT_RE = re.compile(r"(?im)(?<![A-Za-z0-9])(token|password|passwd|api[_-]?key|secret|credential)s?\b[\"']?\s*[:=]\s*[\"']?([^\"'\s#]+)")
 _SECRET_ASSIGNMENT_NAME_RE = re.compile(r"(?i)^(?:token|password|passwd|api[_-]?key|secret|credential)s?$")
+# Model-usage counters share the credential suffix but hold integer measurements.
+_TOKEN_COUNT_NAMES = {"tokens", "total_tokens", "input_tokens", "output_tokens", "prompt_tokens", "completion_tokens", "cached_tokens", "reasoning_tokens"}
 _SECRET_TOKEN_PATTERNS = tuple(
     re.compile(pattern)
     for pattern in (
@@ -400,11 +404,54 @@ def _scan_secret_assignments_by_text(rel_path: str, text: str) -> list[SecurityF
     """``name[:=]value`` sweep for line-oriented text, and for Python that will not parse."""
     for match in _SECRET_ASSIGNMENT_RE.finditer(text):
         value = match.group(2).strip()
+        if _is_token_usage_measurement(text, match):
+            continue
         if not _looks_like_placeholder(value):
             return [_finding_from_match("secret-env-assignment", rel_path, text, match)]
     return []
 
 
+def _is_token_usage_measurement(text: str, match: re.Match[str]) -> bool:
+    """Recognize typed usage counts and benchmark summaries, not credentials."""
+    if match.group(1).lower() != "token" or text[match.end(1) : match.end(1) + 1].lower() != "s":
+        return False
+    start = match.start(1)
+    # Recover the entire key: matching only its suffix would also exempt an
+    # unrelated credential such as access_total_tokens. Scan only this key.
+    while start and (text[start - 1].isalnum() or text[start - 1] in "_.-"):
+        start -= 1
+    end = match.end(1) + 1
+    name = text[start:end].lower()
+    if name not in _TOKEN_COUNT_NAMES:
+        return False
+    if text[end : end + 1] in {'"', "'"} and (not start or text[start - 1] != text[end]):
+        return False
+    value_start = match.start(2)
+    quote = text[value_start - 1] if value_start else ""
+    # The text sweep includes an adjacent JSON delimiter in the value. Require
+    # the whole value to be numeric so strings and trailing payloads stay visible.
+    if quote not in {'"', "'"} and re.fullmatch(r"[0-9]+[,}\]]?", match.group(2)):
+        return True
+    if name != "tokens":
+        return False
+    # Benchmark summaries use four numeric statistics or a signed numeric delta.
+    # Bound parsing independently of file size; other containers remain findings.
+    if quote == '"':
+        value_start -= 1
+    try:
+        value, _ = json.JSONDecoder(object_pairs_hook=tuple).raw_decode(text[value_start : value_start + 512])
+    except ValueError:
+        return False
+    if isinstance(value, str):
+        return re.fullmatch(r"[+-][0-9]+(?:\.[0-9]+)?", value) is not None
+    return (
+        isinstance(value, tuple)
+        and len(value) == 4
+        and {key for key, _ in value} == {"mean", "stddev", "min", "max"}
+        and all(type(number) in {int, float} and number >= 0 and (isinstance(number, int) or math.isfinite(number)) for _, number in value)
+    )
+
+
 def _python_secret_assignment_target(node: ast.expr) -> str | None:
     """Final identifier bound by a simple assignment target, else None."""
     if isinstance(node, ast.Name):

backend/tests/test_skillscan_secret_assignment_formats.py

@@ -8,6 +8,7 @@ to read the same way. The quoted-key shapes below are what a JSON config uses.
 
 from __future__ import annotations
 
+import json
 from pathlib import Path
 
 import pytest
@@ -151,3 +152,63 @@ def test_pretty_printed_json_reports_the_line_the_key_is_on(tmp_path: Path) -> N
     root = _write_package(tmp_path, "config.json", '{\n  "api_key": "' + SECRET + '"\n}')
 
     assert [finding["line"] for finding in _secret_findings(root)] == [2]
+
+
+@pytest.mark.parametrize("name", ["tokens", "total_tokens", "input_tokens", "output_tokens", "prompt_tokens", "completion_tokens", "cached_tokens", "reasoning_tokens"])
+@pytest.mark.parametrize("value", [0, 84852])
+@pytest.mark.parametrize("template", ['{"%s": %s}', '{\n  "%s": %s,\n  "duration_ms": 23332\n}', "%s: %s", "%s=%s"])
+def test_numeric_model_token_counts_are_not_credentials(tmp_path: Path, name: str, value: int, template: str) -> None:
+    root = _write_package(tmp_path, "measurements.md", template % (name.upper() if "=" in template else name, value))
+
+    assert _secret_findings(root) == []
+
+
+@pytest.mark.parametrize(
+    "text",
+    [
+        '{"total_tokens": "84852"}',
+        '{"total_tokens": "' + SECRET + '"}',
+        "total_tokens: 84852secret",
+        "access_tokens: 84852",
+        "api_key: 84852",
+        "password: 84852",
+        "access_total_tokens: 84852",
+        '{"access total_tokens": 84852}',
+        '{"access/total_tokens": 84852}',
+        '{"tokens": "84852"}',
+        '{"tokens": {"mean": "' + SECRET + '", "stddev": 1, "min": 0, "max": 2}}',
+        '{"tokens": {"api_key": "' + SECRET + '"}}',
+        '{"tokens": {"mean": true, "stddev": 1, "min": 0, "max": 2}}',
+        '{"tokens": {"mean": NaN, "stddev": 1, "min": 0, "max": 2}}',
+        '{"tokens": {"mean": 1e999, "stddev": 1, "min": 0, "max": 2}}',
+        '{"tokens": {"mean": "' + SECRET + '", "mean": 1, "stddev": 1, "min": 0, "max": 2}}',
+        '{"tokens": [["mean", 1], ["stddev", 1], ["min", 0], ["max", 2]]}',
+        '{"tokens": "+1700' + SECRET + '"}',
+    ],
+)
+def test_token_count_exemption_preserves_credential_detection(tmp_path: Path, text: str) -> None:
+    root = _write_package(tmp_path, "config.md", text)
+
+    assert [finding["line"] for finding in _secret_findings(root)] == [1]
+
+
+def test_numeric_token_count_does_not_hide_later_credential(tmp_path: Path) -> None:
+    root = _write_package(tmp_path, "config.md", '{"total_tokens": 84852}\napi_key: ' + SECRET)
+
+    assert [finding["line"] for finding in _secret_findings(root)] == [2]
+
+
+def test_numeric_token_count_does_not_suppress_cloud_token_scan(tmp_path: Path) -> None:
+    token = "sk-" + "a1b2c3d4" * 4
+    root = _write_package(tmp_path, "config.md", '{"total_tokens": 84852}\nAuthorization: Bearer ' + token)
+
+    findings = scan_skill_dir(root)["findings"]
+
+    assert any(finding["rule_id"] == "secret-cloud-token" and finding["severity"] == "CRITICAL" for finding in findings)
+
+
+@pytest.mark.parametrize("value", [{"mean": 3800.5, "stddev": 400, "min": 3200, "max": 4100}, "+1700", "-12.5"])
+def test_benchmark_token_summaries_are_measurements(tmp_path: Path, value) -> None:
+    root = _write_package(tmp_path, "benchmark.md", json.dumps({"tokens": value}))
+
+    assert _secret_findings(root) == []

skills/public/skill-creator/references/schemas.md

@@ -297,6 +297,7 @@ Output from Benchmark mode. Located at `benchmarks/<timestamp>/benchmark.json`.
   - `configuration`: Must be `"with_skill"` or `"without_skill"` (the viewer uses this exact string for grouping and color coding)
   - `run_number`: Integer run number (1, 2, 3...)
   - `result`: Nested object with `pass_rate`, `passed`, `total`, `time_seconds`, `tokens`, `errors`
+    - `tokens`: Uses sibling `timing.json.total_tokens` when present (including zero), independently of duration fields in `grading.json`. If the token count is missing or null, aggregation retains the legacy `execution_metrics.output_chars` proxy, defaulting to zero if that is also missing.
 - `run_summary`: Statistical aggregates per configuration
   - `with_skill` / `without_skill`: Each contains `pass_rate`, `time_seconds`, `tokens` objects with `mean` and `stddev` fields
   - `delta`: Difference strings like `"+0.50"`, `"+13.0"`, `"+1700"`

skills/public/skill-creator/scripts/aggregate_benchmark.py

@@ -133,23 +133,25 @@ def load_run_results(benchmark_dir: Path) -> dict:
                     "total": grading.get("summary", {}).get("total", 0),
                 }
 
-                # Extract timing — check grading.json first, then sibling timing.json
+                # Read recorded tokens independently of the grading duration.
                 timing = grading.get("timing", {})
                 result["time_seconds"] = timing.get("total_duration_seconds", 0.0)
                 timing_file = run_dir / "timing.json"
-                if result["time_seconds"] == 0.0 and timing_file.exists():
+                if timing_file.exists():
                     try:
                         with open(timing_file, encoding="utf-8") as tf:
                             timing_data = json.load(tf)
-                        result["time_seconds"] = timing_data.get("total_duration_seconds", 0.0)
-                        result["tokens"] = timing_data.get("total_tokens", 0)
+                        if result["time_seconds"] == 0.0:
+                            result["time_seconds"] = timing_data.get("total_duration_seconds", 0.0)
+                        result["tokens"] = timing_data.get("total_tokens")
                     except json.JSONDecodeError:
                         pass
 
                 # Extract metrics if available
                 metrics = grading.get("execution_metrics", {})
                 result["tool_calls"] = metrics.get("total_tool_calls", 0)
-                if not result.get("tokens"):
+                # Keep the legacy proxy only when no token count was recorded.
+                if result.get("tokens") is None:
                     result["tokens"] = metrics.get("output_chars", 0)
                 result["errors"] = metrics.get("errors_encountered", 0)
 

tests/skills/test_skill_creator_benchmark.py

@@ -0,0 +1,119 @@
+"""Regressions for skill-creator benchmark measurements."""
+
+import json
+import runpy
+from pathlib import Path
+
+import pytest
+
+
+@pytest.fixture
+def benchmark_script():
+    script = (
+        Path(__file__).resolve().parents[2]
+        / "skills/public/skill-creator/scripts/aggregate_benchmark.py"
+    )
+    return runpy.run_path(str(script))
+
+
+def write_run(root, *, grading_timing=None, timing=None, config="with_skill"):
+    run_dir = root / "eval-1" / config / "run-1"
+    run_dir.mkdir(parents=True)
+    grading = {
+        "summary": {"pass_rate": 1.0, "passed": 1, "failed": 0, "total": 1},
+        "execution_metrics": {"output_chars": 12450},
+    }
+    if grading_timing is not None:
+        grading["timing"] = grading_timing
+    (run_dir / "grading.json").write_text(json.dumps(grading), encoding="utf-8")
+    if timing is not None:
+        (run_dir / "timing.json").write_text(json.dumps(timing), encoding="utf-8")
+    return run_dir
+
+
+def test_recorded_tokens_used_when_grading_already_has_duration(
+    tmp_path, benchmark_script
+):
+    write_run(
+        tmp_path,
+        grading_timing={"total_duration_seconds": 23.3},
+        timing={"total_duration_seconds": 25.0, "total_tokens": 84852},
+    )
+
+    benchmark = benchmark_script["generate_benchmark"](tmp_path)
+
+    assert benchmark["runs"][0]["result"]["tokens"] == 84852
+    assert benchmark["runs"][0]["result"]["time_seconds"] == 23.3
+    assert benchmark["run_summary"]["with_skill"]["tokens"]["mean"] == 84852
+
+
+@pytest.mark.parametrize("layout", ["workspace", "legacy"])
+@pytest.mark.parametrize(
+    "grading_timing",
+    [None, {}, {"total_duration_seconds": 0}, {"total_duration_seconds": 23.3}],
+)
+@pytest.mark.parametrize("tokens", [0, 84852])
+def test_recorded_tokens_preserved_in_both_layouts(
+    tmp_path, benchmark_script, layout, grading_timing, tokens
+):
+    root = tmp_path / "runs" if layout == "legacy" else tmp_path
+    write_run(
+        root,
+        grading_timing=grading_timing,
+        timing={"total_duration_seconds": 23.3, "total_tokens": tokens},
+    )
+
+    benchmark = benchmark_script["generate_benchmark"](tmp_path)
+
+    assert benchmark["runs"][0]["result"]["tokens"] == tokens
+    assert benchmark["runs"][0]["result"]["time_seconds"] == 23.3
+    assert benchmark["run_summary"]["with_skill"]["tokens"]["mean"] == tokens
+
+
+@pytest.mark.parametrize(
+    "timing_state", ["absent", "missing_tokens", "null_tokens", "invalid_json"]
+)
+@pytest.mark.parametrize("has_grading_duration", [False, True])
+def test_missing_token_measurements_keep_legacy_character_fallback(
+    tmp_path, benchmark_script, timing_state, has_grading_duration
+):
+    grading_timing = {"total_duration_seconds": 23.3} if has_grading_duration else None
+    timing = None
+    if timing_state in {"missing_tokens", "null_tokens"}:
+        timing = {"total_duration_seconds": 25.0}
+        if timing_state == "null_tokens":
+            timing["total_tokens"] = None
+    run_dir = write_run(tmp_path, grading_timing=grading_timing, timing=timing)
+    if timing_state == "invalid_json":
+        (run_dir / "timing.json").write_text("{invalid", encoding="utf-8")
+
+    benchmark = benchmark_script["generate_benchmark"](tmp_path)
+
+    assert benchmark["runs"][0]["result"]["tokens"] == 12450
+    expected_time = 23.3 if has_grading_duration else (25.0 if timing else 0.0)
+    assert benchmark["runs"][0]["result"]["time_seconds"] == expected_time
+
+
+def test_recorded_tokens_propagate_to_summary_delta_and_markdown(
+    tmp_path, benchmark_script
+):
+    write_run(
+        tmp_path,
+        grading_timing={"total_duration_seconds": 23.3},
+        timing={"total_tokens": 84852},
+        config="with_skill",
+    )
+    write_run(
+        tmp_path,
+        grading_timing={"total_duration_seconds": 20.0},
+        timing={"total_tokens": 10000},
+        config="without_skill",
+    )
+
+    benchmark = benchmark_script["generate_benchmark"](tmp_path)
+
+    assert benchmark["run_summary"]["without_skill"]["tokens"]["mean"] == 10000
+    assert benchmark["run_summary"]["delta"]["tokens"] == "+74852"
+    assert "| Tokens | 84852 ± 0 | 10000 ± 0 | +74852 |" in benchmark_script[
+        "generate_markdown"
+    ](benchmark)