Skip to content

Commit adfe0d5

Browse files
HexSleevesHexSleeves
committed
Merge remote-tracking branch 'upstream/main' into codex/add-llm-token-usage-reporting
Signed-off-by: Jacob LeCoq <lecoqjacob@gmail.com> # Conflicts: # src/skillspector/llm_utils.py # tests/nodes/test_report.py
2 parents 8b45e81 + 11567e8 commit adfe0d5

91 files changed

Lines changed: 12191 additions & 316 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎.env.example‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,9 @@ NVIDIA_INFERENCE_KEY=
1717
# etc.); leave unset for stock api.openai.com.
1818
OPENAI_API_KEY=
1919
OPENAI_BASE_URL=
20+
# Optional provider- and model-dependent reasoning-effort setting. Non-empty values
21+
# are trimmed and passed through unchanged; unset or blank uses the provider default.
22+
SKILLSPECTOR_REASONING_EFFORT=
2023

2124
# For SKILLSPECTOR_PROVIDER=anthropic.
2225
ANTHROPIC_API_KEY=

‎.github/workflows/ci.yml‎

Lines changed: 66 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -30,38 +30,84 @@ concurrency:
3030
group: ci-${{ github.workflow }}-${{ github.ref }}
3131
cancel-in-progress: true
3232

33+
env:
34+
UV_VERSION: "0.10.x"
35+
PYTHON_VERSION: "3.12"
36+
UV_CACHE_DIR: .uv-cache
37+
UV_LINK_MODE: copy
38+
3339
jobs:
34-
lint-and-test:
35-
name: Lint & Test (Python ${{ matrix.python-version }})
40+
changes:
3641
runs-on: ubuntu-latest
37-
# Windows is excluded: the test suite has known path-separator failures
38-
# in build_context that are out of scope for this workflow.
39-
strategy:
40-
fail-fast: false
41-
matrix:
42-
python-version: ["3.12", "3.13", "3.14"]
43-
42+
outputs:
43+
docker: ${{ steps.filter.outputs.docker }}
4444
steps:
4545
- uses: actions/checkout@v4
46+
with:
47+
fetch-depth: 0
48+
- id: filter
49+
env:
50+
BASE_SHA: ${{ github.event.pull_request.base.sha || github.event.before }}
51+
HEAD_SHA: ${{ github.event.pull_request.head.sha || github.sha }}
52+
run: |
53+
if git diff --quiet "$BASE_SHA" "$HEAD_SHA" -- \
54+
.dockerignore .github/workflows/ci.yml .gitlab-ci.yml Dockerfile \
55+
Makefile pyproject.toml uv.lock src tests/docker tests/fixtures/safe_skill; then
56+
echo "docker=false" >> "$GITHUB_OUTPUT"
57+
else
58+
echo "docker=true" >> "$GITHUB_OUTPUT"
59+
fi
4660
61+
lint:
62+
runs-on: ubuntu-latest
63+
steps:
64+
- uses: actions/checkout@v4
4765
- name: Set up uv
4866
# Pinned to a full commit SHA (third-party action); comment tracks the tag.
4967
uses: astral-sh/setup-uv@d4b2f3b6ecc6e67c4457f6d3e41ec42d3d0fcb86 # v5
5068
with:
69+
version: ${{ env.UV_VERSION }}
5170
enable-cache: true
52-
python-version: ${{ matrix.python-version }}
71+
cache-dependency-glob: uv.lock
72+
python-version: ${{ env.PYTHON_VERSION }}
73+
- run: make install-dev
74+
- run: uv run make lint
75+
- run: uv run make format-check
5376

54-
- name: Install dependencies
55-
run: uv sync --all-extras
56-
57-
- name: Lint with ruff
58-
run: uv run ruff check src/ tests/
59-
60-
- name: Check formatting with ruff
61-
run: uv run ruff format --check src/ tests/
77+
test-unit:
78+
runs-on: ubuntu-latest
79+
steps:
80+
- uses: actions/checkout@v4
81+
- name: Set up uv
82+
# Pinned to a full commit SHA (third-party action); comment tracks the tag.
83+
uses: astral-sh/setup-uv@d4b2f3b6ecc6e67c4457f6d3e41ec42d3d0fcb86 # v5
84+
with:
85+
version: ${{ env.UV_VERSION }}
86+
enable-cache: true
87+
cache-dependency-glob: uv.lock
88+
python-version: ${{ env.PYTHON_VERSION }}
89+
- run: make install-dev
90+
- run: uv run skillspector --version
91+
- run: uv run make test-ci
6292

63-
- name: Run unit tests with coverage
64-
run: uv run pytest -m "not integration" --cov=src/skillspector --cov-report=term-missing
93+
docker-smoke:
94+
needs: changes
95+
if: needs.changes.outputs.docker == 'true'
96+
runs-on: ubuntu-latest
97+
steps:
98+
- uses: actions/checkout@v4
99+
- run: docker version
100+
- run: docker info
101+
- run: docker build -t skillspector .
102+
- run: tests/docker/smoke.sh
103+
- if: always()
104+
uses: actions/upload-artifact@v4
105+
with:
106+
name: docker-smoke-reports
107+
path: |
108+
.skillspector-docker-smoke.json
109+
.skillspector-docker-github-smoke.json
110+
if-no-files-found: ignore
65111

66112
dco:
67113
name: DCO Check

‎CHANGELOG.md‎

Lines changed: 400 additions & 0 deletions
Large diffs are not rendered by default.

‎Makefile‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -152,4 +152,3 @@ docker-build:
152152
# Build and smoke test the Docker image
153153
docker-smoke: docker-build
154154
tests/docker/smoke.sh
155-

‎README.md‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,33 @@ skillspector scan ./my-skill/ --format markdown --output report.md
153153
skillspector scan ./my-skill/ --format sarif --output report.sarif
154154
```
155155

156+
### Batch Scanning
157+
158+
Scan entire directories of skills in parallel from `contrib/batch_scan/`:
159+
160+
```bash
161+
python -m contrib.batch_scan.batch_scan ./my-skills/ --no-llm
162+
python -m contrib.batch_scan.batch_scan ./my-skills/ --workers 20 -f json -o report.json
163+
python -m contrib.batch_scan.batch_scan ./tests/fixtures/ -f terminal --workers 20
164+
```
165+
166+
Supports multilingual detection (zh/ja/ko) and terminal/JSON/Markdown output.
167+
168+
For LLM scans with higher concurrency, configure multiple API keys following
169+
[`.env.example`](contrib/batch_scan/.env.example) — the pool improves throughput
170+
and resilience, provided the keys don't share an account-level rate limit.
171+
172+
See the [contrib guide](contrib/batch_scan/docs/) for details.
173+
174+
> **Note on LLM support:** The default configuration targets DeepSeek as the
175+
> cheapest public option. DeepSeek-Chat is
176+
> [expected to sunset](https://api-docs.deepseek.com/), and the contributor
177+
> does not have hardware to test against local models. The batch scanner was
178+
> originally tested with OpenAI-compatible endpoints — DeepSeek's lack of
179+
> structured-output support required manual JSON-parsing patches. If you can
180+
> contribute a more universal backend (Ollama, vLLM, or a different provider),
181+
> PRs are very welcome.
182+
156183
### Suppressing False Positives (baseline)
157184

158185
Suppress known/accepted findings so the risk score reflects only un-triaged
@@ -530,6 +557,7 @@ Issues (2)
530557
| `NVIDIA_INFERENCE_KEY` | Credential for the `nv_build` provider (build.nvidia.com). | Required for LLM analysis when `SKILLSPECTOR_PROVIDER=nv_build` |
531558
| `OPENAI_API_KEY` | Credential for the OpenAI provider (`SKILLSPECTOR_PROVIDER=openai`). Also serves as the tier-2 fallback in the credential waterfall when the active provider returns no credentials. | Required for LLM analysis when `SKILLSPECTOR_PROVIDER=openai` |
532559
| `OPENAI_BASE_URL` | Override the OpenAI endpoint (e.g. point at Ollama). | Optional |
560+
| `SKILLSPECTOR_REASONING_EFFORT` | Optional provider- and model-dependent reasoning-effort setting. Non-empty values are trimmed and passed through unchanged; unset or blank preserves provider-default behavior. | Optional |
533561
| `ANTHROPIC_API_KEY` | Credential for the Anthropic provider (`SKILLSPECTOR_PROVIDER=anthropic`). | Required for LLM analysis when `SKILLSPECTOR_PROVIDER=anthropic` |
534562
| `ANTHROPIC_PROXY_ENDPOINT_URL` | Full endpoint URL for the Anthropic proxy provider (Vertex-style raw-predict). | Required when `SKILLSPECTOR_PROVIDER=anthropic_proxy` |
535563
| `ANTHROPIC_PROXY_API_KEY` | Bearer token for the Anthropic proxy provider. | Required when `SKILLSPECTOR_PROVIDER=anthropic_proxy` |

‎contrib/batch_scan/.env.example‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
# SkillSpector Batch Scanner — DO NOT COMMIT
2+
#
3+
# Copy to the repository root as .env:
4+
# cp contrib/batch_scan/.env.example .env
5+
#
6+
# =============================================================================
7+
# Multi-key pool (recommended for batch scans)
8+
# =============================================================================
9+
#
10+
# Format: key|base_url|model, separated by semicolons.
11+
# Add as many keys as you want — the pool distributes requests across them.
12+
# ⚠️ Only helps if keys don't share an account-level rate limit.
13+
#
14+
SKILLSPECTOR_API_KEYS="sk-or-xxx1|https://api.deepseek.com|deepseek-chat;sk-or-xxx2|https://api.deepseek.com|deepseek-chat;sk-or-xxx3|https://api.openai.com/v1|gpt-5.4"
15+
16+
# Force OpenAI-compatible provider mode
17+
SKILLSPECTOR_PROVIDER=openai
18+
19+
# Single-key fallback (ignored when SKILLSPECTOR_API_KEYS is set)
20+
OPENAI_API_KEY=sk-or-xxxxxxxxxxxxxxxxxxxxxxxx
21+
OPENAI_BASE_URL=https://api.deepseek.com
22+
23+
SKILLSPECTOR_MODEL=deepseek-chat
24+
SKILLSPECTOR_LOG_LEVEL=WARNING

‎contrib/batch_scan/CONTRIBUTING.md‎

Lines changed: 149 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,149 @@
1+
# Contributing — Multilingual Batch Scanner
2+
3+
> For developers who want to set up, test, and extend this module.
4+
5+
---
6+
7+
## Quick Start
8+
9+
```bash
10+
python3 -m venv .venv
11+
source .venv/bin/activate
12+
pip install -e .
13+
cp contrib/batch_scan/.env.example .env # edit with your API keys
14+
```
15+
16+
Verify everything works:
17+
```bash
18+
python -m contrib.batch_scan.batch_scan ./tests/fixtures/ -f terminal --workers 8
19+
```
20+
21+
---
22+
23+
## Project Map
24+
25+
```
26+
contrib/batch_scan/
27+
├── batch_scan.py # CLI entry + ThreadPoolExecutor (start here)
28+
├── runner.py # graph.invoke() wrapper + 7 patches + pool wiring (core)
29+
├── gap_fill.py # GapFillAnalyzer — LLM pass for 8 uncovered rules
30+
├── api_pool.py # ApiKeyPool — multi-key scheduler + 429 backoff
31+
├── detection.py # Unicode script-ratio language detection
32+
├── annotation.py # Finding language-compatibility labels
33+
├── discovery.py # Recursive SKILL.md finder
34+
├── reports.py # Terminal / JSON / Markdown formatters
35+
├── CONTRIBUTING.md # this file
36+
│
37+
├── docs/
38+
│ ├── README.md # user guide — all commands, test commands, reviewer index
39+
│ ├── DESIGN.md # architecture — concurrency, patches, dual-patch mechanism
40+
│ ├── REVIEW_RESPONSE.md # PR #100 review response
41+
│ └── archive/ # deep dives, history, future work, pitfalls
42+
│
43+
└── tests/
44+
├── test_pool_wiring.py # smoke — 3-path pool verification
45+
├── test_monkeypatch_invasiveness.py # thread isolation, scoping (14 tests)
46+
├── test_monkeypatch_fragility.py # guard verification, deep deps (26 tests)
47+
├── docs/
48+
│ ├── TEST_DESIGN.md # WHY each suite was designed
49+
│ ├── TEST_GUIDE.md # WHAT each file covers + run commands
50+
│ └── BUGS_FOUND.md # 16 bugs found & fixed
51+
└── tests-pro/
52+
├── test_api_pool.py # 45 tests — acquire/release/backoff
53+
├── test_gap_fill.py # 41 tests — JSON parsing, prompt building
54+
├── test_runner_patches.py # 24 tests — context manager, patches
55+
├── test_annotation.py # 10 tests — language compatibility
56+
├── random_numbered.py # main entry point (seed=42)
57+
└── mutation_max.py # 30-bug injection framework
58+
```
59+
60+
---
61+
62+
## Running Tests
63+
64+
```bash
65+
# All 164 tests
66+
python contrib/batch_scan/tests/tests-pro/random_numbered.py # 120 unit (seed=42)
67+
python contrib/batch_scan/tests/test_pool_wiring.py # 4 smoke checks
68+
python contrib/batch_scan/tests/test_monkeypatch_invasiveness.py # 14 thematic
69+
python contrib/batch_scan/tests/test_monkeypatch_fragility.py # 26 thematic
70+
71+
# Review-themed only
72+
python -m unittest \
73+
contrib.batch_scan.tests.test_monkeypatch_invasiveness \
74+
contrib.batch_scan.tests.test_monkeypatch_fragility -v
75+
python contrib/batch_scan/tests/test_pool_wiring.py
76+
77+
# Mutation test
78+
python contrib/batch_scan/tests/tests-pro/mutation_max.py
79+
80+
# End-to-end (fixture suite)
81+
python -m contrib.batch_scan.batch_scan ./tests/fixtures/ -f terminal --workers 8
82+
python -m contrib.batch_scan.batch_scan ./tests/fixtures/ -f terminal --workers 8 --no-llm
83+
```
84+
85+
**Three commands catch most regressions:**
86+
```bash
87+
python contrib/batch_scan/tests/tests-pro/random_numbered.py
88+
python contrib/batch_scan/tests/test_pool_wiring.py
89+
python -m contrib.batch_scan.batch_scan ./tests/fixtures/ -f terminal --workers 8
90+
```
91+
92+
---
93+
94+
## Code Conventions
95+
96+
Match SkillSpector upstream exactly:
97+
98+
- **SPDX header** on every `.py` file
99+
- `from __future__ import annotations` as first import
100+
- Imports: stdlib → third-party → `skillspector.*` → relative (`.`)
101+
- `| None` syntax (not `Optional[X]`)
102+
- `frozenset` / `Final` for module-level constants (`UPPER_SNAKE_CASE`)
103+
- Private helpers: `_lower_snake_case`
104+
- `logger = get_logger(__name__)` in every module
105+
- Comments explain **why**, not what
106+
- Docstrings on all public functions and classes
107+
108+
---
109+
110+
## Commit Style
111+
112+
```
113+
fix: wire ApiKeyPool into llm_analyzer_base graph path
114+
feat: add multilingual batch scanner with parallel execution
115+
docs: document dual-patch pool wiring fix
116+
```
117+
118+
- Present-tense, imperative mood
119+
- `Signed-off-by` trailer required (NVIDIA DCO)
120+
- `Co-authored-by` trailer for joint work
121+
122+
---
123+
124+
## Key Design Points
125+
126+
Before modifying code, understand these three:
127+
128+
1. **Dual-patch pool wiring.** `set_api_pool()` patches both `llm_utils.get_chat_model` AND `llm_analyzer_base.get_chat_model`. The latter is necessary because `llm_analyzer_base` imports via `from ... import`, creating a local reference that single-module patching misses. See `docs/archive/PITFALLS.md`.
129+
130+
2. **Instance-attribute injection (not class-attribute).** Patch 1 writes `self.response_schema = None` to instance `__dict__`, not class `__dict__`. Python MRO finds instance attributes first. This is what makes patches thread-safe. Mutating the class attribute causes cross-thread races (this killed V1).
131+
132+
3. **Guard before apply.** `_verify_patch_targets()` checks all 7 patch assumptions before `_apply_patches()` runs. If upstream changes a signature or removes a dependency, the guard raises immediately — patches fail closed, never silently.
133+
134+
Full architecture: `docs/DESIGN.md`.
135+
All pitfalls: `docs/archive/PITFALLS.md`.
136+
137+
---
138+
139+
## Where to Contribute
140+
141+
See `docs/archive/FUTURE_WORK.md` for 12 future directions with effort estimates. High-impact items:
142+
- Checkpoint/resume (prevents data loss on large scans)
143+
- Language detection expansion (9+ languages)
144+
- SARIF output format
145+
- Non-English ground-truth fixtures
146+
147+
---
148+
149+
**Next:** [docs/README.md](docs/README.md) — user guide · [docs/DESIGN.md](docs/DESIGN.md) — architecture · [docs/REVIEW_RESPONSE.md](docs/REVIEW_RESPONSE.md) — PR #100 review response

0 commit comments

Comments
 (0)