feat(benchmark): size --workers from the machine; docs: record upstream 2.13.1 hold

benchmark/README.md

@@ -35,7 +35,7 @@ Or from inside this directory:
 cd benchmark
 uv run benchmark /path/to/MalSkillBench/Dataset
 uv run benchmark .../Dataset/Prompts/indirect-injection -o out.duckdb
-uv run benchmark .../Dataset/Skills/malware --limit 50 --workers 8
+uv run benchmark .../Dataset/Skills/malware --limit 50 --workers 16
 uv run benchmark .../Dataset --no-llm        # static analysis only
 ```
 
@@ -70,7 +70,7 @@ uv run remove-run results.duckdb ab12 9f3 c7 --yes   # several at once, no promp
 | `--no-llm` | static analysis only (no LLM calls) |
 | `--categories` | comma-separated unit categories to scan; default `skill,code`. **Prompts are excluded by default** — they're raw prompt-injection samples, not the repo config files this tool classifies (PI embedded in a skill is still covered via the `skill` units). Pass `--categories skill,code,prompt` to include them. |
 | `--limit N` | cap units **per group** — a unit's parent directory (e.g. `--limit 20` on `Dataset/Skills` keeps 20 from `Skills/malware` *and* 20 from `Skills/benign`); 0 = no cap |
-| `--workers N` | concurrent scan processes (default 8) |
+| `--workers N` | concurrent scan processes; default is sized from the machine (half the RAM at 256 MB per worker, 6 per CPU, capped at 96 -- 96 on a 48 GB / 16-core Mac). Measured steady-state on the same units: 32 workers ~2 units/s, 64 ~2.6-3.0, 96 ~3.7, 128 ~3.1 -- past ~96 Bedrock queues calls (p99 wait ~46s) and slow scans approach `--timeout`. If the run summary reports scans that **lost LLM calls**, the provider is throttling: lower it. |
 | `--run-id` | extend an existing run instead of starting a new one (requires `-o`); re-scans only its unfinished units and appends into the same run |
 | `--from-run` | re-scan the units of a previous run instead of a `DATASET_PATH` (requires `-o`; reads that run from the `-o` DB and appends a new run there). Omit the positional path. |
 | `--failures-only` | with `--from-run`, re-scan only that run's **failed** units (FN, FP, timeouts, and errors) |

benchmark/benchmark/config.py

@@ -12,10 +12,25 @@
 MODEL_ID = "nvidia.nemotron-super-3-120b"
 SCAN_TIMEOUT_SECONDS = 300
 
+# Auto --workers sizing, measured 2026-10-26 on a 48 GB / 16-core Mac: a scan peaks
+# ~150 MB RSS (the largest units ~1 GB) and 96 workers use ~5 cores. Above ~96,
+# Bedrock starts queueing calls (p99 wait 46s at 128) and throughput drops.
+WORKER_RAM_BUDGET_BYTES = 256 * 2**20
+WORKERS_PER_CPU = 6
+MAX_AUTO_WORKERS = 96
+
 # SkillSpector's own "do not install" verdict == this recommendation string.
 MALICIOUS_RECOMMENDATION = "DO_NOT_INSTALL"
 
 
+def default_workers() -> int:
+    """Scan processes this machine can run: the tighter of the RAM and CPU caps."""
+    ram = os.sysconf("SC_PAGE_SIZE") * os.sysconf("SC_PHYS_PAGES")
+    by_ram = ram // 2 // WORKER_RAM_BUDGET_BYTES
+    by_cpu = (os.cpu_count() or 1) * WORKERS_PER_CPU
+    return max(1, min(by_ram, by_cpu, MAX_AUTO_WORKERS))
+
+
 def bedrock_base_url(region: str) -> str:
     """Bedrock endpoint for ``region`` (keeps the URL in sync with an
     ``AWS_REGION`` override, which also drives the minted token's region).

benchmark/benchmark/db.py

@@ -52,7 +52,9 @@
     run_time DOUBLE,
     status TEXT,
     error TEXT,
-    num_issues INTEGER
+    num_issues INTEGER,
+    llm_calls_attempted INTEGER,
+    llm_calls_succeeded INTEGER  -- < attempted: some LLM batches were dropped (e.g. 429s)
 );
 CREATE TABLE issues (
     run_id TEXT,
@@ -109,6 +111,12 @@
 """
 
 
+# Columns added after the first schema; ALTERed onto older DBs when they are opened.
+_MIGRATIONS = (
+    "ALTER TABLE classifications ADD COLUMN IF NOT EXISTS llm_calls_attempted INTEGER",
+    "ALTER TABLE classifications ADD COLUMN IF NOT EXISTS llm_calls_succeeded INTEGER",
+)
+
 # Every table keyed by run_id -- deleting a run means clearing its rows in all of
 # them (there is no FK cascade in the schema).
 _RUN_TABLES = ("runs", "units", "classifications", "issues", "components")
@@ -174,6 +182,8 @@ def open_db(path: pathlib.Path, run_id: str | None):
         con.execute(_SCHEMA)
         con.execute(_EVAL_VIEW)
         return con, set(), False
+    for stmt in _MIGRATIONS:
+        con.execute(stmt)
     if run_id is None:
         return con, set(), False  # append a new run into the existing DB
     if not con.execute("SELECT 1 FROM runs WHERE run_id = ? LIMIT 1", [run_id]).fetchone():
@@ -271,7 +281,9 @@ def record_result(con, run_id: str, res: ScanResult, no_llm: bool) -> None:
     ok = res.scan_status == "ok"
     predicted = (res.risk_recommendation == MALICIOUS_RECOMMENDATION) if ok else None
     con.execute(
-        "INSERT INTO classifications VALUES (?,?,?,?,?,?,?,?,?,?)",
+        "INSERT INTO classifications (run_id, unit_path, classification, is_malicious, "
+        "risk_score, risk_severity, run_time, status, error, num_issues, "
+        "llm_calls_attempted, llm_calls_succeeded) VALUES (?,?,?,?,?,?,?,?,?,?,?,?)",
         [
             run_id,
             u.unit_path,
@@ -283,6 +295,8 @@ def record_result(con, run_id: str, res: ScanResult, no_llm: bool) -> None:
             classification_status(res, no_llm),
             res.error_message,
             res.num_issues,
+            res.llm_calls_attempted,
+            res.llm_calls_succeeded,
         ],
     )
     for idx, issue in enumerate(res.issues):
@@ -337,3 +351,14 @@ def print_summary(con, run_id: str) -> None:
     total = tp + fp + tn + fn
     if total:
         print(f"  accuracy  = {(tp + tn) / total:.3f}  (over {total} scored units)")
+    partial = con.execute(
+        "SELECT count(*) FROM classifications WHERE run_id = ? "
+        "AND llm_calls_succeeded < llm_calls_attempted",
+        [run_id],
+    ).fetchone()[0]
+    if partial:
+        print(
+            f"  {partial} scan(s) lost some LLM calls (llm_calls_succeeded < "
+            "llm_calls_attempted); a jump here vs. the base run suggests throttling -- "
+            "lower --workers"
+        )

benchmark/benchmark/main.py

@@ -9,7 +9,7 @@
 
 Usage:
     benchmark /path/to/MalSkillBench/Dataset
-    benchmark .../Dataset/Skills/malware --limit 50 --workers 8
+    benchmark .../Dataset/Skills/malware --limit 50 --workers 16
     benchmark .../Dataset --no-llm                       # static analysis only
     benchmark .../Dataset -o out.duckdb --run-id abc123   # extend run abc123
     report out.duckdb <run>                    # single-run summary
@@ -31,7 +31,7 @@
 import click
 from tqdm import tqdm
 
-from .config import SCAN_TIMEOUT_SECONDS, configure_run
+from .config import SCAN_TIMEOUT_SECONDS, configure_run, default_workers
 from .dataset_handler import discover
 from .db import (
     delete_runs,
@@ -94,7 +94,13 @@
     help="cap units PER GROUP (a unit's parent directory, e.g. Skills/malware "
     "vs Skills/benign); 0 = no cap",
 )
-@click.option("--workers", type=int, default=8, show_default=True, help="concurrent scan processes")
+@click.option(
+    "--workers",
+    type=click.IntRange(min=1),
+    default=None,
+    help="concurrent scan processes (default: sized from this machine's RAM and CPUs, "
+    "capped at 96). Lower it if the summary reports scans that lost LLM calls.",
+)
 @click.option(
     "--max-tasks-per-child",
     type=int,
@@ -192,6 +198,9 @@ def _run_benchmark(args: SimpleNamespace) -> int:
         print("ERROR: --dataset only applies with --from-run")
         return 2
 
+    if args.workers is None:
+        args.workers = default_workers()
+
     gen_id = uuid.uuid4().hex[:12]
     out_path = pathlib.Path(args.output or f"benchmark_{gen_id}.duckdb").resolve()
 

benchmark/benchmark/models.py

@@ -54,6 +54,8 @@ class ScanResult:
     num_issues: int = 0
     llm_requested: bool = False
     llm_available: bool = False
+    llm_calls_attempted: int | None = None
+    llm_calls_succeeded: int | None = None
     error_message: str | None = None
     issues: list[dict] = field(default_factory=list)
     components: list[dict] = field(default_factory=list)

benchmark/benchmark/runner.py

@@ -75,6 +75,8 @@ def _report_to_result(unit: Unit, report: dict, elapsed: float) -> ScanResult:
         num_issues=len(issues),
         llm_requested=bool(meta.get("llm_requested")),
         llm_available=bool(meta.get("llm_available")),
+        llm_calls_attempted=meta.get("llm_calls_attempted"),
+        llm_calls_succeeded=meta.get("llm_calls_succeeded"),
         issues=issues,
         components=report.get("components", []) or [],
     )

benchmark/tests/test_config.py

@@ -0,0 +1,27 @@
+# SPDX-License-Identifier: Apache-2.0
+"""Tests for the machine-sized --workers default."""
+
+from __future__ import annotations
+
+from benchmark import config
+
+
+def _machine(monkeypatch, ram_gib: int, cpus: int) -> None:
+    sizes = {"SC_PAGE_SIZE": 4096, "SC_PHYS_PAGES": ram_gib * 2**30 // 4096}
+    monkeypatch.setattr(config.os, "sysconf", sizes.__getitem__)
+    monkeypatch.setattr(config.os, "cpu_count", lambda: cpus)
+
+
+def test_default_workers_is_capped_on_a_large_machine(monkeypatch):
+    _machine(monkeypatch, ram_gib=48, cpus=16)
+    assert config.default_workers() == config.MAX_AUTO_WORKERS
+
+
+def test_default_workers_is_ram_bound_on_a_small_machine(monkeypatch):
+    _machine(monkeypatch, ram_gib=8, cpus=16)
+    assert config.default_workers() == 8 * 2**30 // 2 // config.WORKER_RAM_BUDGET_BYTES
+
+
+def test_default_workers_is_cpu_bound_with_few_cores(monkeypatch):
+    _machine(monkeypatch, ram_gib=64, cpus=2)
+    assert config.default_workers() == 2 * config.WORKERS_PER_CPU

benchmark/tests/test_db.py

@@ -61,3 +61,25 @@ def test_delete_runs_noop_on_empty_id_list():
     _seed_run(con, "x")
     delete_runs(con, [])
     assert con.execute("SELECT count(*) FROM runs").fetchone()[0] == 1
+
+
+def test_open_db_adds_llm_call_columns_to_an_older_db(tmp_path):
+    from benchmark.db import open_db
+
+    path = tmp_path / "old.duckdb"
+    con = duckdb.connect(str(path))
+    con.execute(
+        _SCHEMA.replace(",\n    llm_calls_attempted INTEGER", "").replace(
+            ",\n    llm_calls_succeeded INTEGER  -- < attempted: some LLM batches were dropped (e.g. 429s)",
+            "",
+        )
+    )
+    con.execute(_EVAL_VIEW)
+    assert "llm_calls_attempted" not in {
+        r[0] for r in con.execute("DESCRIBE classifications").fetchall()
+    }
+    con.close()
+
+    con, _, _ = open_db(path, None)
+    cols = {r[0] for r in con.execute("DESCRIBE classifications").fetchall()}
+    assert {"llm_calls_attempted", "llm_calls_succeeded"} <= cols

src/skillspector/exaforce/UPSTREAM_UPGRADE_HOLD.md

@@ -0,0 +1,158 @@
+# Upstream upgrades: on indefinite hold
+
+**Decision (2026-10-26):** the fork stays on upstream NVIDIA/SkillSpector
+**2.8.1**. Pulling 2.13.1 was attempted, benchmarked, patched, and
+re-benchmarked; the best result still had **2x the false positives** and
+**45 scan timeouts** against a base that had none, and the patched tree was
+2.7x slower per scan. The pull is parked in a git stash (see the end of this
+file). Nothing from it is on the branch.
+
+Everything below is from `benchmark/malskillbench_agentguard_seed.duckdb`
+(4,844 units: 3,913 benign, 931 malicious; model
+`nvidia.nemotron-super-3-120b` via bedrock-mantle; verdict = `DO_NOT_INSTALL`).
+
+## Runs
+
+| run | tree | correct | FP | FN | errors | precision | recall | F1 | avg scan |
+|---|---|---|---|---|---|---|---|---|---|
+| `efd838e2e656` | **base**: fork on upstream 2.8.1 | **0.876** | **187** | 415 | **0** | 0.734 | 0.554 | 0.632 | 20.5s |
+| `ca1cf8d781dd` | pure upstream 2.13.1, no fork patches | 0.658 | 1241 | 257 | 157 | 0.314 | 0.688 | 0.431 | 99.0s |
+| `3af1174df436` | 2.13.1 + fork patches re-applied as-is | 0.680 | 1347 | 166 | 35 | 0.361 | 0.821 | 0.501 | 49.7s |
+| `aa767af2fb97` | 2.13.1 + fork patches reworked (below) | 0.851 | 376 | 303 | 45 | 0.624 | 0.673 | 0.648 | 55.2s |
+
+"correct" is (TP + TN) / 4,844 with errors counted as wrong. All 2.13.1 runs
+used 96 workers; the base run's worker count was not recorded. `8ed7dfa5fc00`
+was a 1,624-unit partial of the as-is tree and agrees with `3af1174df436`.
+
+The pure-upstream run shows the regression is upstream's, not the fork's: with
+no fork code at all, 2.13.1 flags 1,241 benign skills where 2.8.1 flagged 187.
+
+## What changed upstream (2.8.1 → 2.13.1) and why it hurts
+
+Two independent changes, neither with an upstream opt-out.
+
+### 1. TP4 analyzes Markdown code fences (#421)
+
+TP4 is the LLM "description-behavior mismatch" check in
+`nodes/analyzers/mcp_tool_poisoning.py`. Until 2.13 it ran only when a skill
+shipped real executable files. Since #421 every `bash`/`python`/`js`/`ts`/…
+fence in SKILL.md (and any other Markdown/text file) is extracted and sent to
+the LLM, one batch per fence, as "the implementation". Benign skills are mostly
+SKILL.md with example commands, so the LLM reports a mismatch per fence at
+HIGH / ~0.92 confidence.
+
+| | base | 2.13.1 as-is |
+|---|---|---|
+| benign units with a TP4 HIGH finding | 37 | 2,183 |
+| share of score points *added* to benign units | | 92 % |
+| benign FPs after rescoring with TP4 removed (offline) | | 1,373 → 623 |
+
+Three TP4 occurrences alone contribute ~40 of the 51 points needed for
+`DO_NOT_INSTALL`. This is also why scan time more than doubled: a live
+20-unit check gave a median of 41s with fence scanning on and 10s with it off.
+
+### 2. The meta-analyzer no longer removes anything (#393)
+
+Before 2.13 the meta-analyzer LLM dropped findings it did not confirm
+(MEDIUM/LOW outright; CRITICAL/HIGH kept on a floor). Since 2.13 it only
+annotates: `report.py` scores `state["findings"]`, which merges by id and
+never deletes ("Meta/LLM analysis can enrich canonical objects but cannot
+remove deterministic findings"), and `finalize_ledger` re-selects every
+finding. The fork's `_filter_patches.py` post-filter still ran, but its drops
+never reached the score. Unconfirmed MEDIUM statics that 2.8.1 removed now all
+count: on benign SKILL.md files E1 went 30 → 639 units, RA2 23 → 435, LP3 and
+PE3 similarly. This is why the fork tree and pure upstream scored the same.
+
+## Fix attempts and outcomes
+
+### Attempt A: re-apply the fork's filter patch unchanged (`3af1174df436`)
+
+Rewritten `apply_filter` wrapper for 2.13's stamp-based review outcomes. All
+unit tests passed; end to end it was a no-op for the reason in §2. Result
+indistinguishable from pure upstream (1,347 vs 1,241 FP).
+
+### Attempt B: live isolation of the two causes (40 units, 4 configs)
+
+20 benign units the pull had flipped TN→FP and 20 malicious units it had
+flipped FN→TP, each scanned under four configurations:
+
+| configuration | benign flagged /20 | malware caught /20 |
+|---|---|---|
+| 2.13.1 as-is | 15 | 17 |
+| TP4 Markdown fences off | 9 | 7 |
+| filter drops made effective | 9 | 14 |
+| both | 4 | 7 |
+| filter effective + TP4 follows meta verdict (offline) | 3 | 7 |
+| filter effective + TP4 scored once per unit (offline) | 5 | 11 |
+
+The malware column is a worst case (these are exactly the units the new code
+fixed), but it shows the trade: most of 2.13's recall gain comes from the same
+fence scanning that causes the false positives. The meta-analyzer marked most
+TP4 findings `missing` or `disagreed`; the fork's `semantic` floor kept them.
+
+### Attempt C: reworked patches (`aa767af2fb97`)
+
+1. `apply_filter` wrapper returns every finding in upstream's order but
+   **zeroes the confidence** of the ones it drops (tag `llm-rejected`).
+   `_compute_risk_score` skips confidence ≤ 0, so the drop reaches the verdict
+   while upstream's "always listed" contract holds. Verified by a graph-level
+   test that failed at score 34 before the change and 0 after.
+2. **TP4 removed from the LLM severity floor**, so it follows the
+   meta-analyzer verdict like a static rule.
+
+Full suite green in all three floor modes (16 upstream tests that assert
+"unconfirmed finding is retained" pass again under the new contract; 4 remain
+by-design xfails). Benchmark vs base:
+
+| | base | attempt C | Δ |
+|---|---|---|---|
+| correct | 4242 | 4120 | −122 |
+| false positives | 187 | 376 | +189 |
+| false negatives | 415 | 303 | −112 |
+| errors (all 300s timeouts) | 0 | 45 | +45 |
+| precision | 0.734 | 0.624 | −0.110 |
+| recall | 0.554 | 0.673 | +0.119 |
+| F1 | 0.632 | 0.648 | +0.016 |
+| avg / median / p95 scan | 20.5s / 14.2s / 64.9s | 55.2s / 28.1s / 237.3s | 2.7x / 2.0x / 3.7x |
+
+The residual 241 new false positives decompose as:
+
+- **TP4 the meta-analyzer confirms**: still on 355 benign units, top
+  contributor on 136 of the 241. Both LLMs agree, so no floor policy touches
+  it; only disabling fence scanning or capping TP4's contribution would.
+- **Lost LLM batches**: 114 of the 376 benign FPs had
+  `llm_calls_succeeded < llm_calls_attempted`. A failed meta-analyzer batch
+  keeps every finding fail-closed at full weight (418 scans lost calls at 96
+  workers; see the `llm-failure-amplification` note in the benchmark docs).
+- **Timeouts**: 45 scans hit the 300s cap, all scored wrong.
+
+F1 is slightly up only because the harmonic mean rewards lifting the weaker
+side (recall); on a corpus that is 81 % benign, overall correctness is down.
+
+## What would be needed to lift the hold
+
+Any of these would need its own controlled A/B against `efd838e2e656`
+(same day, replicated; per-unit flips are noisy, judge aggregates only):
+
+- An upstream opt-out for TP4 fence scanning, or a fork patch that empties
+  `_TP4_MARKDOWN_TYPES` (restores 2.8.1 TP4 gating; cuts scan time ~2.5x;
+  gives up most of 2.13's recall gain).
+- Capping TP4 to one scored occurrence per skill (attempt B's offline
+  estimate: FP 5/20, TP 11/20 on the hard sample).
+- Fewer workers so batches stop being dropped, since dropped batches turn
+  directly into false positives under 2.13's fail-closed design.
+- An upstream change to §2 that lets meta-analysis remove or down-weight
+  findings in the public result.
+
+Without at least the TP4 change the 2.13 line is slower and less precise than
+what the fork ships today, and the recall gain does not pay for it.
+
+## Where the attempt lives
+
+The 2.13.1 pull plus attempt C (`_filter_patches.py`, `tests/exaforce/`,
+root `conftest.py`, `docs/superpowers/EXPECTED_TEST_FAILURES.md`) is in
+`git stash` on branch `bump-upstream-version`, entry
+"upstream 2.13.1 pull + fork fix attempt C (on hold)". The benchmark runs
+above are in the DuckDB and are not part of the stash. Fork-side diagnosis
+notes are also in the Claude project memory under
+`upstream-2-13-fp-regression-causes`.