From 326848fdba209ee23dee40520b66eaf67e2ca88b Mon Sep 17 00:00:00 2001 From: will-exaforce Date: Thu, 2 Jul 2026 14:52:04 -0500 Subject: [PATCH 01/11] Remove keys in schema prompt not being used or actively being overwritten. This is to improve llm request/response time and reduce timeout frequency. --- src/skillspector/llm_analyzer_base.py | 4 -- .../analyzers/semantic_developer_intent.py | 3 +- .../analyzers/semantic_quality_policy.py | 3 +- src/skillspector/nodes/meta_analyzer.py | 30 +------------ tests/nodes/test_llm_analyzer_base.py | 44 ++----------------- tests/nodes/test_semantic_quality_policy.py | 7 ++- 6 files changed, 11 insertions(+), 80 deletions(-) diff --git a/src/skillspector/llm_analyzer_base.py b/src/skillspector/llm_analyzer_base.py index c5ab9dce..dad41722 100644 --- a/src/skillspector/llm_analyzer_base.py +++ b/src/skillspector/llm_analyzer_base.py @@ -73,8 +73,6 @@ class LLMFinding(BaseModel): start_line: int = Field(description="Starting line number (>= 1)") end_line: int | None = Field(default=None, description="Ending line number (optional)") confidence: float = Field(default=0.5, description="Confidence score between 0.0 and 1.0") - explanation: str = Field(default="", description="Why this is a finding (2-3 sentences)") - remediation: str = Field(default="", description="Actionable steps to fix the issue") @field_validator("start_line") @classmethod @@ -103,8 +101,6 @@ def to_finding(self, file: str) -> Finding: file=file, start_line=self.start_line, end_line=self.end_line, - explanation=self.explanation, - remediation=self.remediation, ) diff --git a/src/skillspector/nodes/analyzers/semantic_developer_intent.py b/src/skillspector/nodes/analyzers/semantic_developer_intent.py index f51fe8f0..65b37c43 100644 --- a/src/skillspector/nodes/analyzers/semantic_developer_intent.py +++ b/src/skillspector/nodes/analyzers/semantic_developer_intent.py @@ -41,8 +41,7 @@ Skill manifest context: {manifest_section} -Use the rule IDs exactly as listed. Reference the L-prefixed line numbers -when reporting findings. +Use the rule IDs exactly as listed. | Rule ID | Detection | |---------|-----------| diff --git a/src/skillspector/nodes/analyzers/semantic_quality_policy.py b/src/skillspector/nodes/analyzers/semantic_quality_policy.py index 18b48486..bfe25ae1 100644 --- a/src/skillspector/nodes/analyzers/semantic_quality_policy.py +++ b/src/skillspector/nodes/analyzers/semantic_quality_policy.py @@ -43,8 +43,7 @@ file-type scope matches the current file. If a category says "markdown and manifest files only", do NOT report those findings for .py or .sh files. -Use the rule IDs exactly as listed. Reference the L-prefixed line numbers -when reporting findings. +Use the rule IDs exactly as listed. | Rule ID | Category | Applies to | |---------|----------|------------| diff --git a/src/skillspector/nodes/meta_analyzer.py b/src/skillspector/nodes/meta_analyzer.py index 58c5b634..bad6e83f 100644 --- a/src/skillspector/nodes/meta_analyzer.py +++ b/src/skillspector/nodes/meta_analyzer.py @@ -24,7 +24,6 @@ import asyncio import json -from typing import Literal from pydantic import BaseModel, Field, field_validator @@ -77,28 +76,14 @@ def _normalize_confidence(cls, v: object) -> float: v = v / 100.0 return min(1.0, max(0.0, v)) - intent: Literal["malicious", "negligent", "benign"] = Field( - description="Likely intent behind the finding" - ) - impact: Literal["critical", "high", "medium", "low"] = Field( - description="Potential impact if exploited" - ) explanation: str = Field(default="", description="Why this is dangerous (2-3 sentences)") remediation: str = Field(default="", description="How to fix the issue (actionable steps)") -class OverallAssessment(BaseModel): - """Overall risk assessment for the analyzed file.""" - - risk_level: str = Field(description="Overall risk level: LOW, MEDIUM, HIGH, or CRITICAL") - summary: str = Field(description="Brief summary of findings") - - class MetaAnalyzerResult(BaseModel): """Top-level structured response from the meta-analyzer LLM.""" findings: list[MetaAnalyzerFinding] = Field(default_factory=list) - overall_assessment: OverallAssessment | None = None @field_validator("findings", mode="before") @classmethod @@ -112,17 +97,6 @@ def _parse_stringified_findings(cls, v: object) -> object: return parsed if isinstance(parsed, list) else [] return v - @field_validator("overall_assessment", mode="before") - @classmethod - def _parse_stringified_assessment(cls, v: object) -> object: - """LLMs sometimes return nested objects as JSON strings.""" - if isinstance(v, str): - try: - return json.loads(v) - except (json.JSONDecodeError, TypeError): - return None - return v - # --------------------------------------------------------------------------- # Prompt (no JSON format instructions — schema handles the structure) @@ -163,9 +137,7 @@ def _parse_stringified_assessment(cls, v: object) -> object: For each static analysis finding, evaluate: 1. Is this a true vulnerability or a false positive? -2. What is the likely intent (malicious, negligent, or benign)? -3. What is the potential impact if exploited? -4. Does the skill context make this more or less dangerous? +2. Does the skill context make this more or less dangerous? (e.g., "cyanide" in a cooking skill = CRITICAL, in a chemistry education skill = maybe OK) IMPORTANT: Include the start_line from each finding's Location field (the number diff --git a/tests/nodes/test_llm_analyzer_base.py b/tests/nodes/test_llm_analyzer_base.py index e344e654..e37f0fb7 100644 --- a/tests/nodes/test_llm_analyzer_base.py +++ b/tests/nodes/test_llm_analyzer_base.py @@ -712,8 +712,6 @@ def test_to_finding(self) -> None: start_line=10, end_line=12, confidence=0.95, - explanation="Contains API key", - remediation="Use env vars", ) finding = f.to_finding("config.py") assert isinstance(finding, Finding) @@ -722,8 +720,6 @@ def test_to_finding(self) -> None: assert finding.start_line == 10 assert finding.end_line == 12 assert finding.confidence == 0.95 - assert finding.explanation == "Contains API key" - assert finding.remediation == "Use env vars" def test_model_dump(self) -> None: f = LLMFinding( @@ -736,7 +732,7 @@ def test_model_dump(self) -> None: d = f.model_dump() assert d["rule_id"] == "SEC-002" assert d["severity"] == "MEDIUM" - assert d["explanation"] == "" + assert "explanation" not in d assert d["end_line"] is None @@ -748,8 +744,6 @@ def test_valid_finding(self) -> None: pattern_id="E1", is_vulnerability=True, confidence=0.9, - intent="malicious", - impact="high", explanation="Dangerous", remediation="Fix it", ) @@ -764,15 +758,11 @@ def test_confidence_is_clamped(self) -> None: pattern_id="E1", is_vulnerability=True, confidence=1.5, - intent="malicious", - impact="high", ) low = MetaAnalyzerFinding( pattern_id="E1", is_vulnerability=True, confidence=-0.2, - intent="malicious", - impact="high", ) assert high.confidence == 1.0 assert low.confidence == 0.0 @@ -783,20 +773,18 @@ def test_confidence_100_scale_normalized(self) -> None: pattern_id="E1", is_vulnerability=True, confidence=100, - intent="malicious", - impact="high", ) assert f.confidence == pytest.approx(1.0) def test_confidence_75_scale_normalized(self) -> None: f = MetaAnalyzerFinding( - pattern_id="E1", is_vulnerability=True, confidence=75, intent="malicious", impact="high" + pattern_id="E1", is_vulnerability=True, confidence=75 ) assert f.confidence == pytest.approx(0.75) def test_confidence_negative_clamped(self) -> None: f = MetaAnalyzerFinding( - pattern_id="E1", is_vulnerability=True, confidence=-5, intent="malicious", impact="high" + pattern_id="E1", is_vulnerability=True, confidence=-5 ) assert f.confidence == pytest.approx(0.0) @@ -806,18 +794,6 @@ def test_confidence_validation(self) -> None: pattern_id="E1", is_vulnerability=True, confidence="bad", - intent="malicious", - impact="high", - ) - - def test_intent_validation(self) -> None: - with pytest.raises(ValueError): - MetaAnalyzerFinding( - pattern_id="E1", - is_vulnerability=True, - confidence=0.5, - intent="unknown", - impact="high", ) def test_empty_findings(self) -> None: @@ -829,8 +805,6 @@ def test_start_line_optional(self) -> None: pattern_id="E1", is_vulnerability=True, confidence=0.9, - intent="malicious", - impact="high", ) assert f_no_line.start_line is None @@ -839,8 +813,6 @@ def test_start_line_optional(self) -> None: start_line=42, is_vulnerability=True, confidence=0.9, - intent="malicious", - impact="high", ) assert f_with_line.start_line == 42 @@ -849,8 +821,6 @@ def test_model_dump(self) -> None: pattern_id="E2", is_vulnerability=True, confidence=0.8, - intent="negligent", - impact="medium", ) d = f.model_dump() assert d["pattern_id"] == "E2" @@ -1044,8 +1014,6 @@ def test_converts_pydantic_to_dicts(self) -> None: pattern_id="E1", is_vulnerability=True, confidence=0.9, - intent="malicious", - impact="high", explanation="Bad stuff", ), ] @@ -1586,8 +1554,6 @@ def test_run_batches_calls_structured_llm_per_batch(self, mock_get_model: MagicM pattern_id="E1", is_vulnerability=True, confidence=0.9, - intent="malicious", - impact="high", ) ], ) @@ -1631,8 +1597,6 @@ async def test_arun_batches_calls_ainvoke_per_batch(self, mock_get_model: MagicM pattern_id="E1", is_vulnerability=True, confidence=0.9, - intent="malicious", - impact="high", ) ], ) @@ -1665,8 +1629,6 @@ async def test_arun_batches_results_compatible_with_apply_filter( pattern_id="E1", is_vulnerability=True, confidence=0.9, - intent="malicious", - impact="high", explanation="Dangerous", remediation="Fix it", ) diff --git a/tests/nodes/test_semantic_quality_policy.py b/tests/nodes/test_semantic_quality_policy.py index d0e69cc4..2c2f647d 100644 --- a/tests/nodes/test_semantic_quality_policy.py +++ b/tests/nodes/test_semantic_quality_policy.py @@ -494,8 +494,11 @@ def _patched_init(self_inner, *args, **kwargs): assert cred_finding.file == "scripts/helper.py" assert cred_finding.start_line == 5 assert cred_finding.confidence == 0.95 - assert cred_finding.explanation is not None - assert cred_finding.remediation is not None + # Discovery findings no longer carry explanation/remediation — the + # meta-analyzer is the authoritative enrichment stage, so these are + # populated downstream (or from pattern_defaults), not here. + assert cred_finding.explanation is None + assert cred_finding.remediation is None class TestFixtureSafeSkill: From 459f974f63aa8b8847006190cc571fddd8331ffa Mon Sep 17 00:00:00 2001 From: will-exaforce Date: Fri, 10 Jul 2026 07:37:31 -0500 Subject: [PATCH 02/11] docs: design for isolating PR #8 schema-pruning as exaforce monkeypatch Co-Authored-By: Claude Opus 4.8 (1M context) --- ...07-10-monkeypatch-schema-pruning-design.md | 146 ++++++++++++++++++ src/skillspector/exaforce/__init__.py | 0 2 files changed, 146 insertions(+) create mode 100644 docs/superpowers/specs/2026-07-10-monkeypatch-schema-pruning-design.md create mode 100644 src/skillspector/exaforce/__init__.py diff --git a/docs/superpowers/specs/2026-07-10-monkeypatch-schema-pruning-design.md b/docs/superpowers/specs/2026-07-10-monkeypatch-schema-pruning-design.md new file mode 100644 index 00000000..0d621aa4 --- /dev/null +++ b/docs/superpowers/specs/2026-07-10-monkeypatch-schema-pruning-design.md @@ -0,0 +1,146 @@ +# Design: Move PR #8 schema-pruning into an isolated `exaforce` monkeypatch + +- **Date:** 2026-07-10 +- **Branch:** `prune-unused-keys-in-prompt-schemas` (PR #8) +- **Author:** wbeasley + +## Goal + +Reproduce the exact runtime behavior of PR #8 — pruning rarely-used keys out of +the LLM structured-output schemas and prompts to shrink requests and reduce LLM +timeouts — **without editing upstream source files**. The forked change must live +in the fork-only `src/skillspector/exaforce/` package so upstream syncs +(`NVIDIA/SkillSpector`) merge cleanly. + +## What PR #8 changes (the behavior to reproduce) + +| Upstream file | Runtime effect to reproduce | +|---|---| +| `llm_analyzer_base.py` | Remove `explanation`, `remediation` fields from `LLMFinding`; stop passing them in `LLMFinding.to_finding()` | +| `nodes/meta_analyzer.py` | Remove `intent`, `impact` fields from `MetaAnalyzerFinding`; remove `overall_assessment` field + its `_parse_stringified_assessment` validator from `MetaAnalyzerResult` (the `OverallAssessment` class becomes unused/unreferenced); trim the `PER_FILE_ANALYSIS_PROMPT` task list from 4 items to 2 (drop the intent + impact questions) | +| `nodes/analyzers/semantic_developer_intent.py` | Remove the sentence `"Reference the L-prefixed line numbers when reporting findings."` from `ANALYZER_PROMPT` | +| `nodes/analyzers/semantic_quality_policy.py` | Same sentence removal from `ANALYZER_PROMPT` | + +## Approach: in-place monkeypatch, guarded, activated once at import + +### Why in-place mutation (not class replacement) + +Mutating the existing model classes in place preserves object identity, so: +- `LLMAnalyzerBase.response_schema` / `LLMMetaAnalyzer.response_schema` keep + pointing at the same (now-pruned) class objects — no rebinding needed. +- `isinstance(response, LLMAnalysisResult)` in `parse_response` still holds. +- Nothing that imported these classes earlier ends up with a stale reference. + +Verified behavior of the mutation recipe (Pydantic v2): +1. `del Model.model_fields[name]` for each removed field. +2. Pop any dangling `field_validator` decorator for a removed field via + `Model.__pydantic_decorators__.field_validators.pop(, None)`. +3. `Model.model_rebuild(force=True)` on the leaf model **and** on every container + that nests it (e.g. rebuild `LLMFinding` then `LLMAnalysisResult`). + +Confirmed: after this, removed keys disappear from `model_json_schema()` +(including nested `$defs`), from validation, and from `model_dump()`. + +### Prompts + +Prompt bodies are read from module globals at `node()` / analyzer `__init__` +time, so reassigning the module global before those run takes effect. Use a +**guarded targeted `str.replace()`** (assert the target substring is present, +then replace) rather than copying the full prompt text — this avoids duplicating +~100 lines and detects upstream drift. + +### Guards (the key safety property) + +A source-file fork *conflicts loudly* on upstream sync; a monkeypatch instead +*silently goes stale* if upstream renames a field or rewrites a prompt. To +recover the "loud failure" signal: + +- Before removing a field, assert it is present in `model_fields`. +- Before replacing a prompt substring, assert the substring is present. +- On any failed assertion, raise a clear error naming the drifted symbol. + +This turns silent divergence into a fail-fast at process startup. + +### Idempotency + +`apply_patches()` is guarded by a module-level `_PATCHED` flag. The first call +performs all mutations (and runs the drift guards); subsequent calls are no-ops. +This makes repeated imports / explicit test calls safe. + +## Module layout + +``` +src/skillspector/exaforce/ + __init__.py # exposes apply_patches(); holds _PATCHED flag + _schema_patches.py # LLMFinding / MetaAnalyzerFinding / MetaAnalyzerResult field pruning + to_finding + _prompt_patches.py # the three prompt-string edits +``` +(Final file split can collapse to fewer files if trivial; boundary is +schema-patches vs prompt-patches.) + +## Activation + +Append to the end of `src/skillspector/__init__.py` (after the existing +`from skillspector.graph import ...` at line 35, so target modules are loaded): + +```python +from skillspector import exaforce as _exaforce # noqa: E402 +_exaforce.apply_patches() +``` + +This is the **only** upstream-tracked file edited (one idempotent import + +call). It covers CLI, MCP server, the benchmark, and the test suite uniformly, +because any `import skillspector` runs it. + +## Source files: restore to upstream parity + +Revert these 4 files to their pre-PR-#8 (upstream) content, since their pruning +now happens at runtime via `apply_patches()`: +- `src/skillspector/llm_analyzer_base.py` +- `src/skillspector/nodes/meta_analyzer.py` +- `src/skillspector/nodes/analyzers/semantic_developer_intent.py` +- `src/skillspector/nodes/analyzers/semantic_quality_policy.py` + +## Tests + +- **Keep** PR #8's edits to `tests/nodes/test_llm_analyzer_base.py` and + `tests/nodes/test_semantic_quality_policy.py`. Because `__init__.py` patches + globally on import, the test process sees pruned behavior, so these + pruned-expecting assertions pass. (Reverting them would make them fail against + the patched runtime.) These two test files remain the only forked test + artifacts. +- **Add** `tests/exaforce/test_patches.py` (fork-only) that: + - asserts `apply_patches()` removed the expected keys from each model's JSON + schema and from `model_dump()`; + - asserts the three prompt strings no longer contain the removed sentences; + - asserts idempotency (calling `apply_patches()` twice is safe); + - asserts the drift guards raise when a target field/substring is absent + (simulate by monkeypatching a target away, then expecting the guard error). + +## Verification + +- `uv run pytest` green (full suite, incl. kept pruned-expecting tests + new + exaforce tests). +- A one-off runtime check: import `skillspector`, then dump + `LLMAnalysisResult.model_json_schema()` and `MetaAnalyzerResult.model_json_schema()` + and confirm the pruned keys are absent and the prompts lack the removed + sentences — i.e. behavior identical to the PR #8 diff. +- `git diff upstream/main -- src/skillspector/llm_analyzer_base.py src/skillspector/nodes/meta_analyzer.py src/skillspector/nodes/analyzers/semantic_developer_intent.py src/skillspector/nodes/analyzers/semantic_quality_policy.py` + is empty (source files at parity). + +## Out of scope / non-goals + +- No plugin system, no config flag to toggle pruning — pruning is always on + (it is the desired product behavior). +- No reversible/per-test-scoped patching machinery; patches are process-global, + matching the fact that the product always wants the pruned schema. + +## Risks + +1. **Silent drift** if upstream changes a targeted field/prompt — mitigated by + fail-fast guards (raise at startup, don't silently no-op). +2. **Pydantic internal reliance** (`model_fields`, `__pydantic_decorators__`, + `model_rebuild`) could break on a major Pydantic upgrade — mitigated by the + exaforce test suite catching it in CI. +3. **Two test files remain forked** — accepted; their assertions track real + (pruned) behavior and conflict far less than the source files did. diff --git a/src/skillspector/exaforce/__init__.py b/src/skillspector/exaforce/__init__.py new file mode 100644 index 00000000..e69de29b From c86c6bf4142db5cfe57e41fa5643f9caeaeae6bf Mon Sep 17 00:00:00 2001 From: will-exaforce Date: Fri, 10 Jul 2026 07:39:27 -0500 Subject: [PATCH 03/11] docs: revert upstream tests (accept runtime failures) for full parity Co-Authored-By: Claude Opus 4.8 (1M context) --- ...07-10-monkeypatch-schema-pruning-design.md | 27 ++++++++++++------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/docs/superpowers/specs/2026-07-10-monkeypatch-schema-pruning-design.md b/docs/superpowers/specs/2026-07-10-monkeypatch-schema-pruning-design.md index 0d621aa4..07823c00 100644 --- a/docs/superpowers/specs/2026-07-10-monkeypatch-schema-pruning-design.md +++ b/docs/superpowers/specs/2026-07-10-monkeypatch-schema-pruning-design.md @@ -103,12 +103,14 @@ now happens at runtime via `apply_patches()`: ## Tests -- **Keep** PR #8's edits to `tests/nodes/test_llm_analyzer_base.py` and - `tests/nodes/test_semantic_quality_policy.py`. Because `__init__.py` patches - globally on import, the test process sees pruned behavior, so these - pruned-expecting assertions pass. (Reverting them would make them fail against - the patched runtime.) These two test files remain the only forked test - artifacts. +- **Revert** PR #8's edits to `tests/nodes/test_llm_analyzer_base.py` and + `tests/nodes/test_semantic_quality_policy.py` back to upstream parity. Because + `__init__.py` patches globally on import, these upstream tests assert the + **un-pruned** schema (e.g. `d["explanation"] == ""`, `test_intent_validation` + expecting a `ValueError`) and will therefore **fail** at runtime. This is an + accepted, deliberate trade: keeping every upstream file at parity is worth more + than a green upstream test suite, and avoids the maintenance burden of keeping + forked test assertions in sync with upstream. - **Add** `tests/exaforce/test_patches.py` (fork-only) that: - asserts `apply_patches()` removed the expected keys from each model's JSON schema and from `model_dump()`; @@ -119,8 +121,10 @@ now happens at runtime via `apply_patches()`: ## Verification -- `uv run pytest` green (full suite, incl. kept pruned-expecting tests + new - exaforce tests). +- `uv run pytest` passes **except** for the reverted upstream tests that assert + un-pruned behavior (a known, documented set of failures — see Tests). The new + `tests/exaforce/` suite and everything else pass. Record the exact expected + failures so a reviewer can distinguish them from new regressions. - A one-off runtime check: import `skillspector`, then dump `LLMAnalysisResult.model_json_schema()` and `MetaAnalyzerResult.model_json_schema()` and confirm the pruned keys are absent and the prompts lack the removed @@ -142,5 +146,8 @@ now happens at runtime via `apply_patches()`: 2. **Pydantic internal reliance** (`model_fields`, `__pydantic_decorators__`, `model_rebuild`) could break on a major Pydantic upgrade — mitigated by the exaforce test suite catching it in CI. -3. **Two test files remain forked** — accepted; their assertions track real - (pruned) behavior and conflict far less than the source files did. +3. **Reverted upstream tests fail at runtime** — accepted. Cost: CI/`pytest` + signal is polluted, since a genuinely new regression in those files would be + harder to spot among the known failures. Mitigated by documenting the exact + expected-failure set. Chosen deliberately over the maintenance burden of + forked test assertions. From 733ad83b4b2c2aeb1810da6c98a9fe9930ecf3ea Mon Sep 17 00:00:00 2001 From: will-exaforce Date: Fri, 10 Jul 2026 07:49:33 -0500 Subject: [PATCH 04/11] docs: implementation plan for exaforce monkeypatch schema-pruning Co-Authored-By: Claude Opus 4.8 (1M context) --- ...-10-exaforce-monkeypatch-schema-pruning.md | 652 ++++++++++++++++++ 1 file changed, 652 insertions(+) create mode 100644 docs/superpowers/plans/2026-07-10-exaforce-monkeypatch-schema-pruning.md diff --git a/docs/superpowers/plans/2026-07-10-exaforce-monkeypatch-schema-pruning.md b/docs/superpowers/plans/2026-07-10-exaforce-monkeypatch-schema-pruning.md new file mode 100644 index 00000000..b2f5aab7 --- /dev/null +++ b/docs/superpowers/plans/2026-07-10-exaforce-monkeypatch-schema-pruning.md @@ -0,0 +1,652 @@ +# ExaForce Monkeypatch Schema-Pruning Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Reproduce PR #8's LLM schema/prompt pruning at runtime from the fork-only `src/skillspector/exaforce/` package, and restore every upstream-tracked file it edited to upstream parity. + +**Architecture:** A guarded monkeypatch. On `import skillspector`, `exaforce.apply_patches()` mutates the existing Pydantic model classes in place (`del model_fields[...]` + `model_rebuild(force=True)` on leaf and container, preserving class identity) and rewrites three prompt module-globals via targeted `str.replace()`. Every mutation asserts its upstream target still exists and raises `PatchDriftError` if not, so an upstream rename fails loudly instead of silently going stale. + +**Tech Stack:** Python 3.x, Pydantic v2, pytest, `uv` for running everything. + +## Global Constraints + +- Only `src/skillspector/exaforce/**` and **one** appended block in `src/skillspector/__init__.py` may diverge from upstream. Every other upstream-tracked file (source and tests) must end at upstream (pre-PR-#8) parity. Pre-PR-#8 tree = commit `bec96da` (`326848f^`). +- All guards must **raise** (`PatchDriftError`) on drift — never silently no-op a mutation. +- `apply_patches()` must be idempotent (safe to call repeatedly). +- Do **not** touch unrelated working-tree changes already present (`benchmark/**`, `.claude/**`, `.superpowers/**`, `docs/superpowers/**`, `*.db`, `*_report.md`, etc.). Stage only the exact files each task names. +- Run all commands via `uv run`. Test suite command: `uv run pytest`. +- The scanned skill content is adversarial input; do not change any security invariant — only reproduce PR #8's key/prompt pruning. + +--- + +### Task 1: Revert PR #8 files to upstream parity + +Undo PR #8's edits so the four source files are un-pruned again (the patch in later tasks needs those fields/prompts present to remove), and the two test files return to upstream. After this task the tree equals pre-PR-#8 and the suite is green. + +**Files:** +- Modify (revert): `src/skillspector/llm_analyzer_base.py` +- Modify (revert): `src/skillspector/nodes/meta_analyzer.py` +- Modify (revert): `src/skillspector/nodes/analyzers/semantic_developer_intent.py` +- Modify (revert): `src/skillspector/nodes/analyzers/semantic_quality_policy.py` +- Modify (revert): `tests/nodes/test_llm_analyzer_base.py` +- Modify (revert): `tests/nodes/test_semantic_quality_policy.py` + +**Interfaces:** +- Produces (restored upstream symbols later tasks patch): `llm_analyzer_base.LLMFinding` (with fields `explanation`, `remediation` and a `to_finding` that forwards them), `llm_analyzer_base.LLMAnalysisResult`; `meta_analyzer.MetaAnalyzerFinding` (with `intent`, `impact`), `meta_analyzer.MetaAnalyzerResult` (with `overall_assessment` field + `_parse_stringified_assessment` field_validator), `meta_analyzer.PER_FILE_ANALYSIS_PROMPT`; `semantic_developer_intent.ANALYZER_PROMPT`; `semantic_quality_policy.ANALYZER_PROMPT`. + +- [ ] **Step 1: Restore the six files from the pre-PR-#8 commit** + +```bash +git checkout 326848f^ -- \ + src/skillspector/llm_analyzer_base.py \ + src/skillspector/nodes/meta_analyzer.py \ + src/skillspector/nodes/analyzers/semantic_developer_intent.py \ + src/skillspector/nodes/analyzers/semantic_quality_policy.py \ + tests/nodes/test_llm_analyzer_base.py \ + tests/nodes/test_semantic_quality_policy.py +``` + +- [ ] **Step 2: Verify the four source files now contain the upstream (un-pruned) symbols** + +Run: +```bash +git diff 326848f^ -- \ + src/skillspector/llm_analyzer_base.py \ + src/skillspector/nodes/meta_analyzer.py \ + src/skillspector/nodes/analyzers/semantic_developer_intent.py \ + src/skillspector/nodes/analyzers/semantic_quality_policy.py \ + tests/nodes/test_llm_analyzer_base.py \ + tests/nodes/test_semantic_quality_policy.py +``` +Expected: **no output** (working tree matches pre-PR-#8 for these files). + +Also confirm the fields are back: +```bash +grep -n "explanation\|remediation" src/skillspector/llm_analyzer_base.py +grep -n "intent\|impact\|overall_assessment\|OverallAssessment" src/skillspector/nodes/meta_analyzer.py +``` +Expected: matches present (fields restored). + +- [ ] **Step 3: Run the suite to confirm the reverted baseline is green** + +Run: `uv run pytest -q` +Expected: PASS (source and tests are both un-pruned, so consistent). Note the passing count for comparison in Task 4. + +- [ ] **Step 4: Commit** + +```bash +git add \ + src/skillspector/llm_analyzer_base.py \ + src/skillspector/nodes/meta_analyzer.py \ + src/skillspector/nodes/analyzers/semantic_developer_intent.py \ + src/skillspector/nodes/analyzers/semantic_quality_policy.py \ + tests/nodes/test_llm_analyzer_base.py \ + tests/nodes/test_semantic_quality_policy.py +git commit -m "revert: restore PR #8 files to upstream parity + +Pruning moves to the fork-only exaforce runtime patch (subsequent tasks)." +``` + +--- + +### Task 2: exaforce guard primitives + unit tests + +Add the small, dependency-free patch primitives used by every later patch, with the drift guards. No real models are touched yet — these are tested against throwaway models. + +**Files:** +- Create: `src/skillspector/exaforce/_patchlib.py` +- Create: `tests/exaforce/__init__.py` (empty) +- Test: `tests/exaforce/test_patchlib.py` + +Note: `src/skillspector/exaforce/__init__.py` already exists (empty); leave it for Task 3. + +**Interfaces:** +- Produces: + - `class PatchDriftError(RuntimeError)` + - `remove_model_fields(model: type[BaseModel], field_names: list[str]) -> None` — deletes each name from `model.model_fields`; raises `PatchDriftError` if a name is absent. Caller must `model_rebuild(force=True)` afterward. + - `pop_field_validator(model: type[BaseModel], validator_name: str) -> None` — removes a field_validator decorator by function name; no-op if absent. + - `replace_module_str(module: ModuleType, attr: str, old: str, new: str) -> None` — sets `module.attr = module.attr.replace(old, new)`; raises `PatchDriftError` if `old` not present. + +- [ ] **Step 1: Write the failing tests** + +Create `tests/exaforce/__init__.py` (empty file), then `tests/exaforce/test_patchlib.py`: + +```python +from types import ModuleType + +import pytest +from pydantic import BaseModel + +from skillspector.exaforce._patchlib import ( + PatchDriftError, + pop_field_validator, + remove_model_fields, + replace_module_str, +) + + +def test_remove_model_fields_removes_then_rebuild_drops_key(): + class M(BaseModel): + a: str + b: str = "" + + remove_model_fields(M, ["b"]) + M.model_rebuild(force=True) + assert "b" not in M.model_json_schema()["properties"] + assert "a" in M.model_json_schema()["properties"] + + +def test_remove_model_fields_raises_on_missing_field(): + class M(BaseModel): + a: str + + with pytest.raises(PatchDriftError): + remove_model_fields(M, ["nope"]) + + +def test_pop_field_validator_is_noop_when_absent(): + class M(BaseModel): + a: str + + pop_field_validator(M, "nonexistent") # must not raise + + +def test_replace_module_str_replaces_substring(): + mod = ModuleType("dummy") + mod.P = "hello world" + replace_module_str(mod, "P", "world", "there") + assert mod.P == "hello there" + + +def test_replace_module_str_raises_on_missing_substring(): + mod = ModuleType("dummy") + mod.P = "hello world" + with pytest.raises(PatchDriftError): + replace_module_str(mod, "P", "absent", "x") +``` + +- [ ] **Step 2: Run the tests to verify they fail** + +Run: `uv run pytest tests/exaforce/test_patchlib.py -q` +Expected: FAIL with `ModuleNotFoundError`/`ImportError` (`_patchlib` does not exist yet). + +- [ ] **Step 3: Implement `_patchlib.py`** + +Create `src/skillspector/exaforce/_patchlib.py`: + +```python +# SPDX-License-Identifier: Apache-2.0 +"""Guarded monkeypatch primitives for the ExaForce fork. + +Each primitive asserts its upstream target still exists before mutating it, so +an upstream rename/rewrite fails loudly (``PatchDriftError``) at import time +instead of silently leaving the fork's runtime behavior stale. +""" + +from __future__ import annotations + +from types import ModuleType + +from pydantic import BaseModel + + +class PatchDriftError(RuntimeError): + """An upstream target a fork patch depends on has changed or is missing.""" + + +def remove_model_fields(model: type[BaseModel], field_names: list[str]) -> None: + """Delete ``field_names`` from ``model.model_fields``. + + The caller must run ``model.model_rebuild(force=True)`` afterward (and on any + container model that nests ``model``) for the change to reach the emitted + JSON schema. Raises ``PatchDriftError`` if a field is absent. + """ + for name in field_names: + if name not in model.model_fields: + raise PatchDriftError( + f"{model.__module__}.{model.__qualname__} has no field {name!r}; " + "upstream changed — update the exaforce patch." + ) + del model.model_fields[name] + + +def pop_field_validator(model: type[BaseModel], validator_name: str) -> None: + """Remove a ``field_validator`` decorator by its function name (no-op if absent).""" + model.__pydantic_decorators__.field_validators.pop(validator_name, None) + + +def replace_module_str(module: ModuleType, attr: str, old: str, new: str) -> None: + """Replace substring ``old`` with ``new`` in module-global ``attr``. + + Raises ``PatchDriftError`` if ``old`` is not present in the current value. + """ + current = getattr(module, attr) + if old not in current: + raise PatchDriftError( + f"{module.__name__}.{attr} does not contain the expected text; " + "upstream changed — update the exaforce patch." + ) + setattr(module, attr, current.replace(old, new)) +``` + +- [ ] **Step 4: Run the tests to verify they pass** + +Run: `uv run pytest tests/exaforce/test_patchlib.py -q` +Expected: PASS (5 passed). + +- [ ] **Step 5: Commit** + +```bash +git add src/skillspector/exaforce/_patchlib.py tests/exaforce/__init__.py tests/exaforce/test_patchlib.py +git commit -m "feat(exaforce): guarded monkeypatch primitives" +``` + +--- + +### Task 3: Schema + prompt patches and `apply_patches()` + +Implement the actual pruning patches and the idempotent `apply_patches()` orchestrator. **No activation from `skillspector/__init__.py` yet** — this task tests via a subprocess that imports skillspector and calls `apply_patches()` explicitly, so the in-process suite (and the reverted upstream tests) stay green. + +**Files:** +- Create: `src/skillspector/exaforce/_schema_patches.py` +- Create: `src/skillspector/exaforce/_prompt_patches.py` +- Modify: `src/skillspector/exaforce/__init__.py` (currently empty) +- Test: `tests/exaforce/test_patches.py` + +**Interfaces:** +- Consumes: `PatchDriftError`, `remove_model_fields`, `pop_field_validator`, `replace_module_str` from `_patchlib` (Task 2); the restored upstream symbols from Task 1. +- Produces: + - `skillspector.exaforce.apply_patches() -> None` — idempotent; runs `_schema_patches.apply()` then `_prompt_patches.apply()`. + - `_schema_patches.apply() -> None`, `_prompt_patches.apply() -> None`. + +- [ ] **Step 1: Write the failing tests** + +Create `tests/exaforce/test_patches.py`: + +```python +import subprocess +import sys +import textwrap + + +def _run(code: str) -> str: + """Run *code* in a fresh interpreter; return stdout, assert clean exit.""" + result = subprocess.run( + [sys.executable, "-c", textwrap.dedent(code)], + capture_output=True, + text=True, + ) + assert result.returncode == 0, result.stderr + return result.stdout + + +def test_apply_patches_prunes_model_schemas(): + out = _run( + """ + import skillspector + from skillspector.exaforce import apply_patches + apply_patches() + from skillspector.llm_analyzer_base import LLMFinding, LLMAnalysisResult + from skillspector.nodes.meta_analyzer import ( + MetaAnalyzerFinding, + MetaAnalyzerResult, + ) + lf = set(LLMFinding.model_json_schema()["properties"]) + assert "explanation" not in lf and "remediation" not in lf, lf + mf = set(MetaAnalyzerFinding.model_json_schema()["properties"]) + assert "intent" not in mf and "impact" not in mf, mf + mr = set(MetaAnalyzerResult.model_json_schema()["properties"]) + assert "overall_assessment" not in mr, mr + # Container schema (what the LLM actually receives) is pruned too: + nested = LLMAnalysisResult.model_json_schema()["$defs"]["LLMFinding"]["properties"] + assert "explanation" not in nested, nested + print("OK") + """ + ) + assert "OK" in out + + +def test_apply_patches_prunes_to_finding_and_dump(): + out = _run( + """ + import skillspector + from skillspector.exaforce import apply_patches + apply_patches() + from skillspector.llm_analyzer_base import LLMFinding + f = LLMFinding(rule_id="R", message="m", severity="LOW", start_line=3) + assert "explanation" not in f.model_dump() + assert "remediation" not in f.model_dump() + fin = f.to_finding("x.py") + assert fin.explanation is None + assert fin.rule_id == "R" and fin.start_line == 3 + print("OK") + """ + ) + assert "OK" in out + + +def test_apply_patches_trims_prompts(): + out = _run( + """ + import skillspector + from skillspector.exaforce import apply_patches + apply_patches() + from skillspector.nodes.analyzers import semantic_developer_intent as d + from skillspector.nodes.analyzers import semantic_quality_policy as q + from skillspector.nodes import meta_analyzer as m + assert "Reference the L-prefixed line numbers" not in d.ANALYZER_PROMPT + assert "Reference the L-prefixed line numbers" not in q.ANALYZER_PROMPT + assert "What is the likely intent" not in m.PER_FILE_ANALYSIS_PROMPT + assert "What is the potential impact" not in m.PER_FILE_ANALYSIS_PROMPT + assert "Use the rule IDs exactly as listed." in d.ANALYZER_PROMPT + print("OK") + """ + ) + assert "OK" in out + + +def test_apply_patches_is_idempotent(): + out = _run( + """ + import skillspector + from skillspector.exaforce import apply_patches + apply_patches() + apply_patches() + apply_patches() + print("OK") + """ + ) + assert "OK" in out +``` + +- [ ] **Step 2: Run the tests to verify they fail** + +Run: `uv run pytest tests/exaforce/test_patches.py -q` +Expected: FAIL — subprocess exits non-zero because `apply_patches` is not importable yet (empty `exaforce/__init__.py`), so `_run` hits its `assert result.returncode == 0`. + +- [ ] **Step 3: Implement `_schema_patches.py`** + +Create `src/skillspector/exaforce/_schema_patches.py`: + +```python +# SPDX-License-Identifier: Apache-2.0 +"""Prune unused keys from the LLM structured-output schemas (fork behavior). + +Reproduces the schema pruning that would otherwise live in upstream +``llm_analyzer_base`` / ``meta_analyzer``, keeping those upstream files at +parity with NVIDIA/SkillSpector. +""" + +from __future__ import annotations + +import skillspector.llm_analyzer_base as llm_base +import skillspector.nodes.meta_analyzer as meta +from skillspector.models import Finding + +from ._patchlib import pop_field_validator, remove_model_fields + + +def _pruned_to_finding(self: "llm_base.LLMFinding", file: str) -> Finding: + """``LLMFinding.to_finding`` without the removed explanation/remediation.""" + return Finding( + rule_id=self.rule_id, + message=self.message, + severity=self.severity, + confidence=self.confidence, + file=file, + start_line=self.start_line, + end_line=self.end_line, + ) + + +def apply() -> None: + # LLMFinding: drop explanation + remediation, and stop forwarding them. + remove_model_fields(llm_base.LLMFinding, ["explanation", "remediation"]) + llm_base.LLMFinding.to_finding = _pruned_to_finding + llm_base.LLMFinding.model_rebuild(force=True) + llm_base.LLMAnalysisResult.model_rebuild(force=True) + + # MetaAnalyzerFinding: drop intent + impact. + remove_model_fields(meta.MetaAnalyzerFinding, ["intent", "impact"]) + meta.MetaAnalyzerFinding.model_rebuild(force=True) + + # MetaAnalyzerResult: drop overall_assessment field + its validator. + remove_model_fields(meta.MetaAnalyzerResult, ["overall_assessment"]) + pop_field_validator(meta.MetaAnalyzerResult, "_parse_stringified_assessment") + meta.MetaAnalyzerResult.model_rebuild(force=True) +``` + +- [ ] **Step 4: Implement `_prompt_patches.py`** + +Create `src/skillspector/exaforce/_prompt_patches.py`: + +```python +# SPDX-License-Identifier: Apache-2.0 +"""Trim prompt text the pruned schema no longer needs (fork behavior).""" + +from __future__ import annotations + +import skillspector.nodes.analyzers.semantic_developer_intent as dev_intent +import skillspector.nodes.analyzers.semantic_quality_policy as quality +import skillspector.nodes.meta_analyzer as meta + +from ._patchlib import replace_module_str + +# Present verbatim in both semantic analyzers' ANALYZER_PROMPT (note the two +# spaces after "listed." and the mid-sentence newline). +_LINE_NUMBER_OLD = ( + "Use the rule IDs exactly as listed. Reference the L-prefixed line numbers\n" + "when reporting findings." +) +_LINE_NUMBER_NEW = "Use the rule IDs exactly as listed." + +# The meta-analyzer's "Your Task" list: drop the intent (2) and impact (3) items. +_META_TASK_OLD = ( + "1. Is this a true vulnerability or a false positive?\n" + "2. What is the likely intent (malicious, negligent, or benign)?\n" + "3. What is the potential impact if exploited?\n" + "4. Does the skill context make this more or less dangerous?" +) +_META_TASK_NEW = ( + "1. Is this a true vulnerability or a false positive?\n" + "2. Does the skill context make this more or less dangerous?" +) + + +def apply() -> None: + replace_module_str(dev_intent, "ANALYZER_PROMPT", _LINE_NUMBER_OLD, _LINE_NUMBER_NEW) + replace_module_str(quality, "ANALYZER_PROMPT", _LINE_NUMBER_OLD, _LINE_NUMBER_NEW) + replace_module_str(meta, "PER_FILE_ANALYSIS_PROMPT", _META_TASK_OLD, _META_TASK_NEW) +``` + +- [ ] **Step 5: Implement `exaforce/__init__.py`** + +Overwrite `src/skillspector/exaforce/__init__.py`: + +```python +# SPDX-License-Identifier: Apache-2.0 +"""ExaForce fork-local runtime patches. + +Keeps fork behavior — pruning unused LLM structured-output keys and prompt text +to shrink requests and reduce LLM timeouts — out of upstream-tracked source +files. All mutations are guarded: an upstream rename/rewrite raises +``PatchDriftError`` at import time rather than silently going stale. +""" + +from __future__ import annotations + +from . import _prompt_patches, _schema_patches + +_PATCHED = False + + +def apply_patches() -> None: + """Idempotently apply all fork-local runtime patches.""" + global _PATCHED + if _PATCHED: + return + _schema_patches.apply() + _prompt_patches.apply() + _PATCHED = True +``` + +- [ ] **Step 6: Run the tests to verify they pass** + +Run: `uv run pytest tests/exaforce/test_patches.py -q` +Expected: PASS (4 passed). + +- [ ] **Step 7: Confirm the in-process suite is still green (no activation yet)** + +Run: `uv run pytest -q` +Expected: PASS — same count as Task 1 Step 3 plus the new exaforce tests. The reverted upstream tests still pass because `apply_patches()` has not been wired into `import skillspector` yet (it only runs inside the subprocess tests). + +- [ ] **Step 8: Commit** + +```bash +git add src/skillspector/exaforce/_schema_patches.py \ + src/skillspector/exaforce/_prompt_patches.py \ + src/skillspector/exaforce/__init__.py \ + tests/exaforce/test_patches.py +git commit -m "feat(exaforce): prune LLM schema keys + prompt text via guarded patch" +``` + +--- + +### Task 4: Activate on import + document expected upstream-test failures + +Wire `apply_patches()` into `skillspector/__init__.py` so the pruning is the default everywhere (CLI, MCP, benchmark, tests). This makes the runtime match PR #8 exactly. As an accepted consequence, the two reverted upstream test files now fail (they assert the un-pruned schema); capture that exact set. + +**Files:** +- Modify: `src/skillspector/__init__.py` (append activation block after `__all__`) +- Create: `tests/exaforce/test_activation.py` +- Create: `docs/superpowers/EXPECTED_TEST_FAILURES.md` + +**Interfaces:** +- Consumes: `skillspector.exaforce.apply_patches` (Task 3). +- Produces: bare `import skillspector` yields pruned schemas/prompts. + +- [ ] **Step 1: Write the failing activation test** + +Create `tests/exaforce/test_activation.py`: + +```python +import subprocess +import sys +import textwrap + + +def _run(code: str) -> str: + result = subprocess.run( + [sys.executable, "-c", textwrap.dedent(code)], + capture_output=True, + text=True, + ) + assert result.returncode == 0, result.stderr + return result.stdout + + +def test_bare_import_activates_patches(): + # No explicit apply_patches() call — importing skillspector must patch. + out = _run( + """ + import skillspector + from skillspector.llm_analyzer_base import LLMFinding + from skillspector.nodes.meta_analyzer import ( + MetaAnalyzerFinding, + MetaAnalyzerResult, + ) + from skillspector.nodes import meta_analyzer as m + assert "explanation" not in LLMFinding.model_json_schema()["properties"] + assert "intent" not in MetaAnalyzerFinding.model_json_schema()["properties"] + assert "overall_assessment" not in MetaAnalyzerResult.model_json_schema()["properties"] + assert "What is the likely intent" not in m.PER_FILE_ANALYSIS_PROMPT + print("OK") + """ + ) + assert "OK" in out +``` + +- [ ] **Step 2: Run it to verify it fails** + +Run: `uv run pytest tests/exaforce/test_activation.py -q` +Expected: FAIL — the subprocess assertion trips (bare import does not patch yet), so `_run` sees a non-zero exit. + +- [ ] **Step 3: Append the activation block to `skillspector/__init__.py`** + +Add these two lines at the **end** of `src/skillspector/__init__.py` (after the existing `__all__ = [...]` line — by this point `from skillspector.graph import ...` has already imported the target modules): + +```python + +# ExaForce fork: apply runtime schema/prompt patches (kept out of upstream files). +from skillspector import exaforce as _exaforce # noqa: E402 +_exaforce.apply_patches() +``` + +- [ ] **Step 4: Run the activation test to verify it passes** + +Run: `uv run pytest tests/exaforce/test_activation.py -q` +Expected: PASS (1 passed). + +- [ ] **Step 5: Capture the expected upstream-test failures** + +Run (the `- true` keeps going despite pytest's non-zero exit on failures): +```bash +uv run pytest tests/nodes/test_llm_analyzer_base.py tests/nodes/test_semantic_quality_policy.py -q || true +``` +Expected: some FAILED tests in exactly these two files (assertions expecting `explanation`/`remediation`/`intent`/`impact`/`overall_assessment`). Confirm each failure is an **assertion** about a pruned key — **not** an import/collection error (which would indicate a real bug). + +- [ ] **Step 6: Record the failure set** + +Create `docs/superpowers/EXPECTED_TEST_FAILURES.md` listing the failing node IDs from Step 5, e.g.: + +```markdown +# Expected test failures (fork: exaforce schema pruning) + +These upstream tests are kept at upstream parity on purpose and therefore +assert the *un-pruned* schema, which the exaforce runtime patch removes. They +are expected to FAIL. A failure here is only a problem if the failure is NOT an +assertion about a pruned key (e.g. an import/collection error). + + +- tests/nodes/test_llm_analyzer_base.py::... +- tests/nodes/test_semantic_quality_policy.py::... +``` + +- [ ] **Step 7: Confirm the failure set is bounded to those two files** + +Run: `uv run pytest -q -rf || true` +Expected: the only FAILED tests are inside `tests/nodes/test_llm_analyzer_base.py` and `tests/nodes/test_semantic_quality_policy.py`. The `tests/exaforce/**` suite passes. If any *other* file fails, investigate before finishing (it means the monkeypatch changed behavior beyond PR #8's scope). + +- [ ] **Step 8: Confirm upstream-file parity (the whole point of the fork)** + +Run: +```bash +git diff 326848f^ -- \ + src/skillspector/llm_analyzer_base.py \ + src/skillspector/nodes/meta_analyzer.py \ + src/skillspector/nodes/analyzers/semantic_developer_intent.py \ + src/skillspector/nodes/analyzers/semantic_quality_policy.py \ + tests/nodes/test_llm_analyzer_base.py \ + tests/nodes/test_semantic_quality_policy.py +``` +Expected: **no output**. And `git diff 326848f^ -- src/skillspector/__init__.py` shows only the two-line activation block added. + +- [ ] **Step 9: Commit** + +```bash +git add src/skillspector/__init__.py tests/exaforce/test_activation.py docs/superpowers/EXPECTED_TEST_FAILURES.md +git commit -m "feat(exaforce): activate schema-pruning patch on import + +Runtime now matches PR #8. The two reverted upstream test files assert the +un-pruned schema and fail by design (see docs/superpowers/EXPECTED_TEST_FAILURES.md)." +``` + +--- + +## Post-plan verification + +- [ ] `uv run pytest tests/exaforce -q` → all pass. +- [ ] `uv run pytest -q -rf` → the only failures are in the two reverted upstream test files, matching `docs/superpowers/EXPECTED_TEST_FAILURES.md`. +- [ ] Behavioral equivalence to PR #8: in a fresh interpreter, `import skillspector` then dumping `LLMAnalysisResult.model_json_schema()` and `MetaAnalyzerResult.model_json_schema()` shows the same pruned key set PR #8 produced, and the three prompts lack the removed sentences. +- [ ] `git diff 326848f^` touches only `src/skillspector/exaforce/**`, the two-line block in `src/skillspector/__init__.py`, `tests/exaforce/**`, and `docs/superpowers/**` — no other upstream-tracked file diverges. From ddb61c83efd3749eb36503f33cef3d5e00d51452 Mon Sep 17 00:00:00 2001 From: will-exaforce Date: Fri, 10 Jul 2026 08:06:09 -0500 Subject: [PATCH 05/11] docs: factor subprocess test helper into shared fixture in plan Co-Authored-By: Claude Opus 4.8 (1M context) --- ...-10-exaforce-monkeypatch-schema-pruning.md | 69 +++++++++---------- 1 file changed, 33 insertions(+), 36 deletions(-) diff --git a/docs/superpowers/plans/2026-07-10-exaforce-monkeypatch-schema-pruning.md b/docs/superpowers/plans/2026-07-10-exaforce-monkeypatch-schema-pruning.md index b2f5aab7..8d594666 100644 --- a/docs/superpowers/plans/2026-07-10-exaforce-monkeypatch-schema-pruning.md +++ b/docs/superpowers/plans/2026-07-10-exaforce-monkeypatch-schema-pruning.md @@ -251,6 +251,7 @@ Implement the actual pruning patches and the idempotent `apply_patches()` orches - Create: `src/skillspector/exaforce/_schema_patches.py` - Create: `src/skillspector/exaforce/_prompt_patches.py` - Modify: `src/skillspector/exaforce/__init__.py` (currently empty) +- Create: `tests/exaforce/conftest.py` (shared `run_in_subprocess` fixture, reused by Task 4) - Test: `tests/exaforce/test_patches.py` **Interfaces:** @@ -259,29 +260,39 @@ Implement the actual pruning patches and the idempotent `apply_patches()` orches - `skillspector.exaforce.apply_patches() -> None` — idempotent; runs `_schema_patches.apply()` then `_prompt_patches.apply()`. - `_schema_patches.apply() -> None`, `_prompt_patches.apply() -> None`. -- [ ] **Step 1: Write the failing tests** +- [ ] **Step 1: Write the shared fixture and the failing tests** -Create `tests/exaforce/test_patches.py`: +Create `tests/exaforce/conftest.py` (the subprocess runner both this task and Task 4 use — a fresh interpreter is how we assert the *real* import-time behavior without polluting the in-process patch state): ```python import subprocess import sys import textwrap +import pytest + -def _run(code: str) -> str: +@pytest.fixture +def run_in_subprocess(): """Run *code* in a fresh interpreter; return stdout, assert clean exit.""" - result = subprocess.run( - [sys.executable, "-c", textwrap.dedent(code)], - capture_output=True, - text=True, - ) - assert result.returncode == 0, result.stderr - return result.stdout + def _run(code: str) -> str: + result = subprocess.run( + [sys.executable, "-c", textwrap.dedent(code)], + capture_output=True, + text=True, + ) + assert result.returncode == 0, result.stderr + return result.stdout -def test_apply_patches_prunes_model_schemas(): - out = _run( + return _run +``` + +Then create `tests/exaforce/test_patches.py`: + +```python +def test_apply_patches_prunes_model_schemas(run_in_subprocess): + out = run_in_subprocess( """ import skillspector from skillspector.exaforce import apply_patches @@ -306,8 +317,8 @@ def test_apply_patches_prunes_model_schemas(): assert "OK" in out -def test_apply_patches_prunes_to_finding_and_dump(): - out = _run( +def test_apply_patches_prunes_to_finding_and_dump(run_in_subprocess): + out = run_in_subprocess( """ import skillspector from skillspector.exaforce import apply_patches @@ -325,8 +336,8 @@ def test_apply_patches_prunes_to_finding_and_dump(): assert "OK" in out -def test_apply_patches_trims_prompts(): - out = _run( +def test_apply_patches_trims_prompts(run_in_subprocess): + out = run_in_subprocess( """ import skillspector from skillspector.exaforce import apply_patches @@ -345,8 +356,8 @@ def test_apply_patches_trims_prompts(): assert "OK" in out -def test_apply_patches_is_idempotent(): - out = _run( +def test_apply_patches_is_idempotent(run_in_subprocess): + out = run_in_subprocess( """ import skillspector from skillspector.exaforce import apply_patches @@ -506,6 +517,7 @@ Expected: PASS — same count as Task 1 Step 3 plus the new exaforce tests. The git add src/skillspector/exaforce/_schema_patches.py \ src/skillspector/exaforce/_prompt_patches.py \ src/skillspector/exaforce/__init__.py \ + tests/exaforce/conftest.py \ tests/exaforce/test_patches.py git commit -m "feat(exaforce): prune LLM schema keys + prompt text via guarded patch" ``` @@ -527,27 +539,12 @@ Wire `apply_patches()` into `skillspector/__init__.py` so the pruning is the def - [ ] **Step 1: Write the failing activation test** -Create `tests/exaforce/test_activation.py`: +Create `tests/exaforce/test_activation.py` (reuses the `run_in_subprocess` fixture from `tests/exaforce/conftest.py`, added in Task 3): ```python -import subprocess -import sys -import textwrap - - -def _run(code: str) -> str: - result = subprocess.run( - [sys.executable, "-c", textwrap.dedent(code)], - capture_output=True, - text=True, - ) - assert result.returncode == 0, result.stderr - return result.stdout - - -def test_bare_import_activates_patches(): +def test_bare_import_activates_patches(run_in_subprocess): # No explicit apply_patches() call — importing skillspector must patch. - out = _run( + out = run_in_subprocess( """ import skillspector from skillspector.llm_analyzer_base import LLMFinding From a2ddf77460b211abd2496cb7b69597c11590f21e Mon Sep 17 00:00:00 2001 From: will-exaforce Date: Fri, 10 Jul 2026 08:07:17 -0500 Subject: [PATCH 06/11] revert: restore PR #8 files to upstream parity Pruning moves to the fork-only exaforce runtime patch (subsequent tasks). --- src/skillspector/llm_analyzer_base.py | 4 ++ .../analyzers/semantic_developer_intent.py | 3 +- .../analyzers/semantic_quality_policy.py | 3 +- src/skillspector/nodes/meta_analyzer.py | 30 ++++++++++++- tests/nodes/test_llm_analyzer_base.py | 44 +++++++++++++++++-- tests/nodes/test_semantic_quality_policy.py | 7 +-- 6 files changed, 80 insertions(+), 11 deletions(-) diff --git a/src/skillspector/llm_analyzer_base.py b/src/skillspector/llm_analyzer_base.py index dad41722..c5ab9dce 100644 --- a/src/skillspector/llm_analyzer_base.py +++ b/src/skillspector/llm_analyzer_base.py @@ -73,6 +73,8 @@ class LLMFinding(BaseModel): start_line: int = Field(description="Starting line number (>= 1)") end_line: int | None = Field(default=None, description="Ending line number (optional)") confidence: float = Field(default=0.5, description="Confidence score between 0.0 and 1.0") + explanation: str = Field(default="", description="Why this is a finding (2-3 sentences)") + remediation: str = Field(default="", description="Actionable steps to fix the issue") @field_validator("start_line") @classmethod @@ -101,6 +103,8 @@ def to_finding(self, file: str) -> Finding: file=file, start_line=self.start_line, end_line=self.end_line, + explanation=self.explanation, + remediation=self.remediation, ) diff --git a/src/skillspector/nodes/analyzers/semantic_developer_intent.py b/src/skillspector/nodes/analyzers/semantic_developer_intent.py index 65b37c43..f51fe8f0 100644 --- a/src/skillspector/nodes/analyzers/semantic_developer_intent.py +++ b/src/skillspector/nodes/analyzers/semantic_developer_intent.py @@ -41,7 +41,8 @@ Skill manifest context: {manifest_section} -Use the rule IDs exactly as listed. +Use the rule IDs exactly as listed. Reference the L-prefixed line numbers +when reporting findings. | Rule ID | Detection | |---------|-----------| diff --git a/src/skillspector/nodes/analyzers/semantic_quality_policy.py b/src/skillspector/nodes/analyzers/semantic_quality_policy.py index bfe25ae1..18b48486 100644 --- a/src/skillspector/nodes/analyzers/semantic_quality_policy.py +++ b/src/skillspector/nodes/analyzers/semantic_quality_policy.py @@ -43,7 +43,8 @@ file-type scope matches the current file. If a category says "markdown and manifest files only", do NOT report those findings for .py or .sh files. -Use the rule IDs exactly as listed. +Use the rule IDs exactly as listed. Reference the L-prefixed line numbers +when reporting findings. | Rule ID | Category | Applies to | |---------|----------|------------| diff --git a/src/skillspector/nodes/meta_analyzer.py b/src/skillspector/nodes/meta_analyzer.py index bad6e83f..58c5b634 100644 --- a/src/skillspector/nodes/meta_analyzer.py +++ b/src/skillspector/nodes/meta_analyzer.py @@ -24,6 +24,7 @@ import asyncio import json +from typing import Literal from pydantic import BaseModel, Field, field_validator @@ -76,14 +77,28 @@ def _normalize_confidence(cls, v: object) -> float: v = v / 100.0 return min(1.0, max(0.0, v)) + intent: Literal["malicious", "negligent", "benign"] = Field( + description="Likely intent behind the finding" + ) + impact: Literal["critical", "high", "medium", "low"] = Field( + description="Potential impact if exploited" + ) explanation: str = Field(default="", description="Why this is dangerous (2-3 sentences)") remediation: str = Field(default="", description="How to fix the issue (actionable steps)") +class OverallAssessment(BaseModel): + """Overall risk assessment for the analyzed file.""" + + risk_level: str = Field(description="Overall risk level: LOW, MEDIUM, HIGH, or CRITICAL") + summary: str = Field(description="Brief summary of findings") + + class MetaAnalyzerResult(BaseModel): """Top-level structured response from the meta-analyzer LLM.""" findings: list[MetaAnalyzerFinding] = Field(default_factory=list) + overall_assessment: OverallAssessment | None = None @field_validator("findings", mode="before") @classmethod @@ -97,6 +112,17 @@ def _parse_stringified_findings(cls, v: object) -> object: return parsed if isinstance(parsed, list) else [] return v + @field_validator("overall_assessment", mode="before") + @classmethod + def _parse_stringified_assessment(cls, v: object) -> object: + """LLMs sometimes return nested objects as JSON strings.""" + if isinstance(v, str): + try: + return json.loads(v) + except (json.JSONDecodeError, TypeError): + return None + return v + # --------------------------------------------------------------------------- # Prompt (no JSON format instructions — schema handles the structure) @@ -137,7 +163,9 @@ def _parse_stringified_findings(cls, v: object) -> object: For each static analysis finding, evaluate: 1. Is this a true vulnerability or a false positive? -2. Does the skill context make this more or less dangerous? +2. What is the likely intent (malicious, negligent, or benign)? +3. What is the potential impact if exploited? +4. Does the skill context make this more or less dangerous? (e.g., "cyanide" in a cooking skill = CRITICAL, in a chemistry education skill = maybe OK) IMPORTANT: Include the start_line from each finding's Location field (the number diff --git a/tests/nodes/test_llm_analyzer_base.py b/tests/nodes/test_llm_analyzer_base.py index e37f0fb7..e344e654 100644 --- a/tests/nodes/test_llm_analyzer_base.py +++ b/tests/nodes/test_llm_analyzer_base.py @@ -712,6 +712,8 @@ def test_to_finding(self) -> None: start_line=10, end_line=12, confidence=0.95, + explanation="Contains API key", + remediation="Use env vars", ) finding = f.to_finding("config.py") assert isinstance(finding, Finding) @@ -720,6 +722,8 @@ def test_to_finding(self) -> None: assert finding.start_line == 10 assert finding.end_line == 12 assert finding.confidence == 0.95 + assert finding.explanation == "Contains API key" + assert finding.remediation == "Use env vars" def test_model_dump(self) -> None: f = LLMFinding( @@ -732,7 +736,7 @@ def test_model_dump(self) -> None: d = f.model_dump() assert d["rule_id"] == "SEC-002" assert d["severity"] == "MEDIUM" - assert "explanation" not in d + assert d["explanation"] == "" assert d["end_line"] is None @@ -744,6 +748,8 @@ def test_valid_finding(self) -> None: pattern_id="E1", is_vulnerability=True, confidence=0.9, + intent="malicious", + impact="high", explanation="Dangerous", remediation="Fix it", ) @@ -758,11 +764,15 @@ def test_confidence_is_clamped(self) -> None: pattern_id="E1", is_vulnerability=True, confidence=1.5, + intent="malicious", + impact="high", ) low = MetaAnalyzerFinding( pattern_id="E1", is_vulnerability=True, confidence=-0.2, + intent="malicious", + impact="high", ) assert high.confidence == 1.0 assert low.confidence == 0.0 @@ -773,18 +783,20 @@ def test_confidence_100_scale_normalized(self) -> None: pattern_id="E1", is_vulnerability=True, confidence=100, + intent="malicious", + impact="high", ) assert f.confidence == pytest.approx(1.0) def test_confidence_75_scale_normalized(self) -> None: f = MetaAnalyzerFinding( - pattern_id="E1", is_vulnerability=True, confidence=75 + pattern_id="E1", is_vulnerability=True, confidence=75, intent="malicious", impact="high" ) assert f.confidence == pytest.approx(0.75) def test_confidence_negative_clamped(self) -> None: f = MetaAnalyzerFinding( - pattern_id="E1", is_vulnerability=True, confidence=-5 + pattern_id="E1", is_vulnerability=True, confidence=-5, intent="malicious", impact="high" ) assert f.confidence == pytest.approx(0.0) @@ -794,6 +806,18 @@ def test_confidence_validation(self) -> None: pattern_id="E1", is_vulnerability=True, confidence="bad", + intent="malicious", + impact="high", + ) + + def test_intent_validation(self) -> None: + with pytest.raises(ValueError): + MetaAnalyzerFinding( + pattern_id="E1", + is_vulnerability=True, + confidence=0.5, + intent="unknown", + impact="high", ) def test_empty_findings(self) -> None: @@ -805,6 +829,8 @@ def test_start_line_optional(self) -> None: pattern_id="E1", is_vulnerability=True, confidence=0.9, + intent="malicious", + impact="high", ) assert f_no_line.start_line is None @@ -813,6 +839,8 @@ def test_start_line_optional(self) -> None: start_line=42, is_vulnerability=True, confidence=0.9, + intent="malicious", + impact="high", ) assert f_with_line.start_line == 42 @@ -821,6 +849,8 @@ def test_model_dump(self) -> None: pattern_id="E2", is_vulnerability=True, confidence=0.8, + intent="negligent", + impact="medium", ) d = f.model_dump() assert d["pattern_id"] == "E2" @@ -1014,6 +1044,8 @@ def test_converts_pydantic_to_dicts(self) -> None: pattern_id="E1", is_vulnerability=True, confidence=0.9, + intent="malicious", + impact="high", explanation="Bad stuff", ), ] @@ -1554,6 +1586,8 @@ def test_run_batches_calls_structured_llm_per_batch(self, mock_get_model: MagicM pattern_id="E1", is_vulnerability=True, confidence=0.9, + intent="malicious", + impact="high", ) ], ) @@ -1597,6 +1631,8 @@ async def test_arun_batches_calls_ainvoke_per_batch(self, mock_get_model: MagicM pattern_id="E1", is_vulnerability=True, confidence=0.9, + intent="malicious", + impact="high", ) ], ) @@ -1629,6 +1665,8 @@ async def test_arun_batches_results_compatible_with_apply_filter( pattern_id="E1", is_vulnerability=True, confidence=0.9, + intent="malicious", + impact="high", explanation="Dangerous", remediation="Fix it", ) diff --git a/tests/nodes/test_semantic_quality_policy.py b/tests/nodes/test_semantic_quality_policy.py index 2c2f647d..d0e69cc4 100644 --- a/tests/nodes/test_semantic_quality_policy.py +++ b/tests/nodes/test_semantic_quality_policy.py @@ -494,11 +494,8 @@ def _patched_init(self_inner, *args, **kwargs): assert cred_finding.file == "scripts/helper.py" assert cred_finding.start_line == 5 assert cred_finding.confidence == 0.95 - # Discovery findings no longer carry explanation/remediation — the - # meta-analyzer is the authoritative enrichment stage, so these are - # populated downstream (or from pattern_defaults), not here. - assert cred_finding.explanation is None - assert cred_finding.remediation is None + assert cred_finding.explanation is not None + assert cred_finding.remediation is not None class TestFixtureSafeSkill: From 82e8fc222efda41124cae9609192cb21dacd7a35 Mon Sep 17 00:00:00 2001 From: will-exaforce Date: Fri, 10 Jul 2026 08:10:51 -0500 Subject: [PATCH 07/11] feat(exaforce): guarded monkeypatch primitives --- src/skillspector/exaforce/_patchlib.py | 52 ++++++++++++++++++++++++++ tests/exaforce/__init__.py | 0 tests/exaforce/test_patchlib.py | 51 +++++++++++++++++++++++++ 3 files changed, 103 insertions(+) create mode 100644 src/skillspector/exaforce/_patchlib.py create mode 100644 tests/exaforce/__init__.py create mode 100644 tests/exaforce/test_patchlib.py diff --git a/src/skillspector/exaforce/_patchlib.py b/src/skillspector/exaforce/_patchlib.py new file mode 100644 index 00000000..1505c9f7 --- /dev/null +++ b/src/skillspector/exaforce/_patchlib.py @@ -0,0 +1,52 @@ +# SPDX-License-Identifier: Apache-2.0 +"""Guarded monkeypatch primitives for the ExaForce fork. + +Each primitive asserts its upstream target still exists before mutating it, so +an upstream rename/rewrite fails loudly (``PatchDriftError``) at import time +instead of silently leaving the fork's runtime behavior stale. +""" + +from __future__ import annotations + +from types import ModuleType + +from pydantic import BaseModel + + +class PatchDriftError(RuntimeError): + """An upstream target a fork patch depends on has changed or is missing.""" + + +def remove_model_fields(model: type[BaseModel], field_names: list[str]) -> None: + """Delete ``field_names`` from ``model.model_fields``. + + The caller must run ``model.model_rebuild(force=True)`` afterward (and on any + container model that nests ``model``) for the change to reach the emitted + JSON schema. Raises ``PatchDriftError`` if a field is absent. + """ + for name in field_names: + if name not in model.model_fields: + raise PatchDriftError( + f"{model.__module__}.{model.__qualname__} has no field {name!r}; " + "upstream changed — update the exaforce patch." + ) + del model.model_fields[name] + + +def pop_field_validator(model: type[BaseModel], validator_name: str) -> None: + """Remove a ``field_validator`` decorator by its function name (no-op if absent).""" + model.__pydantic_decorators__.field_validators.pop(validator_name, None) + + +def replace_module_str(module: ModuleType, attr: str, old: str, new: str) -> None: + """Replace substring ``old`` with ``new`` in module-global ``attr``. + + Raises ``PatchDriftError`` if ``old`` is not present in the current value. + """ + current = getattr(module, attr) + if old not in current: + raise PatchDriftError( + f"{module.__name__}.{attr} does not contain the expected text; " + "upstream changed — update the exaforce patch." + ) + setattr(module, attr, current.replace(old, new)) diff --git a/tests/exaforce/__init__.py b/tests/exaforce/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/tests/exaforce/test_patchlib.py b/tests/exaforce/test_patchlib.py new file mode 100644 index 00000000..dda79dc2 --- /dev/null +++ b/tests/exaforce/test_patchlib.py @@ -0,0 +1,51 @@ +from types import ModuleType + +import pytest +from pydantic import BaseModel + +from skillspector.exaforce._patchlib import ( + PatchDriftError, + pop_field_validator, + remove_model_fields, + replace_module_str, +) + + +def test_remove_model_fields_removes_then_rebuild_drops_key(): + class M(BaseModel): + a: str + b: str = "" + + remove_model_fields(M, ["b"]) + M.model_rebuild(force=True) + assert "b" not in M.model_json_schema()["properties"] + assert "a" in M.model_json_schema()["properties"] + + +def test_remove_model_fields_raises_on_missing_field(): + class M(BaseModel): + a: str + + with pytest.raises(PatchDriftError): + remove_model_fields(M, ["nope"]) + + +def test_pop_field_validator_is_noop_when_absent(): + class M(BaseModel): + a: str + + pop_field_validator(M, "nonexistent") # must not raise + + +def test_replace_module_str_replaces_substring(): + mod = ModuleType("dummy") + mod.P = "hello world" + replace_module_str(mod, "P", "world", "there") + assert mod.P == "hello there" + + +def test_replace_module_str_raises_on_missing_substring(): + mod = ModuleType("dummy") + mod.P = "hello world" + with pytest.raises(PatchDriftError): + replace_module_str(mod, "P", "absent", "x") From a2f6d978b10abeb6cc268b53eb23a5d68e225951 Mon Sep 17 00:00:00 2001 From: will-exaforce Date: Fri, 10 Jul 2026 08:15:07 -0500 Subject: [PATCH 08/11] feat(exaforce): prune LLM schema keys + prompt text via guarded patch --- src/skillspector/exaforce/__init__.py | 24 ++++++ src/skillspector/exaforce/_prompt_patches.py | 36 +++++++++ src/skillspector/exaforce/_schema_patches.py | 45 ++++++++++++ tests/exaforce/conftest.py | 21 ++++++ tests/exaforce/test_patches.py | 77 ++++++++++++++++++++ 5 files changed, 203 insertions(+) create mode 100644 src/skillspector/exaforce/_prompt_patches.py create mode 100644 src/skillspector/exaforce/_schema_patches.py create mode 100644 tests/exaforce/conftest.py create mode 100644 tests/exaforce/test_patches.py diff --git a/src/skillspector/exaforce/__init__.py b/src/skillspector/exaforce/__init__.py index e69de29b..15b7b27f 100644 --- a/src/skillspector/exaforce/__init__.py +++ b/src/skillspector/exaforce/__init__.py @@ -0,0 +1,24 @@ +# SPDX-License-Identifier: Apache-2.0 +"""ExaForce fork-local runtime patches. + +Keeps fork behavior — pruning unused LLM structured-output keys and prompt text +to shrink requests and reduce LLM timeouts — out of upstream-tracked source +files. All mutations are guarded: an upstream rename/rewrite raises +``PatchDriftError`` at import time rather than silently going stale. +""" + +from __future__ import annotations + +from . import _prompt_patches, _schema_patches + +_PATCHED = False + + +def apply_patches() -> None: + """Idempotently apply all fork-local runtime patches.""" + global _PATCHED + if _PATCHED: + return + _schema_patches.apply() + _prompt_patches.apply() + _PATCHED = True diff --git a/src/skillspector/exaforce/_prompt_patches.py b/src/skillspector/exaforce/_prompt_patches.py new file mode 100644 index 00000000..77e68991 --- /dev/null +++ b/src/skillspector/exaforce/_prompt_patches.py @@ -0,0 +1,36 @@ +# SPDX-License-Identifier: Apache-2.0 +"""Trim prompt text the pruned schema no longer needs (fork behavior).""" + +from __future__ import annotations + +import skillspector.nodes.analyzers.semantic_developer_intent as dev_intent +import skillspector.nodes.analyzers.semantic_quality_policy as quality +import skillspector.nodes.meta_analyzer as meta + +from ._patchlib import replace_module_str + +# Present verbatim in both semantic analyzers' ANALYZER_PROMPT (note the two +# spaces after "listed." and the mid-sentence newline). +_LINE_NUMBER_OLD = ( + "Use the rule IDs exactly as listed. Reference the L-prefixed line numbers\n" + "when reporting findings." +) +_LINE_NUMBER_NEW = "Use the rule IDs exactly as listed." + +# The meta-analyzer's "Your Task" list: drop the intent (2) and impact (3) items. +_META_TASK_OLD = ( + "1. Is this a true vulnerability or a false positive?\n" + "2. What is the likely intent (malicious, negligent, or benign)?\n" + "3. What is the potential impact if exploited?\n" + "4. Does the skill context make this more or less dangerous?" +) +_META_TASK_NEW = ( + "1. Is this a true vulnerability or a false positive?\n" + "2. Does the skill context make this more or less dangerous?" +) + + +def apply() -> None: + replace_module_str(dev_intent, "ANALYZER_PROMPT", _LINE_NUMBER_OLD, _LINE_NUMBER_NEW) + replace_module_str(quality, "ANALYZER_PROMPT", _LINE_NUMBER_OLD, _LINE_NUMBER_NEW) + replace_module_str(meta, "PER_FILE_ANALYSIS_PROMPT", _META_TASK_OLD, _META_TASK_NEW) diff --git a/src/skillspector/exaforce/_schema_patches.py b/src/skillspector/exaforce/_schema_patches.py new file mode 100644 index 00000000..5d8c6c45 --- /dev/null +++ b/src/skillspector/exaforce/_schema_patches.py @@ -0,0 +1,45 @@ +# SPDX-License-Identifier: Apache-2.0 +"""Prune unused keys from the LLM structured-output schemas (fork behavior). + +Reproduces the schema pruning that would otherwise live in upstream +``llm_analyzer_base`` / ``meta_analyzer``, keeping those upstream files at +parity with NVIDIA/SkillSpector. +""" + +from __future__ import annotations + +import skillspector.llm_analyzer_base as llm_base +import skillspector.nodes.meta_analyzer as meta +from skillspector.models import Finding + +from ._patchlib import pop_field_validator, remove_model_fields + + +def _pruned_to_finding(self: "llm_base.LLMFinding", file: str) -> Finding: + """``LLMFinding.to_finding`` without the removed explanation/remediation.""" + return Finding( + rule_id=self.rule_id, + message=self.message, + severity=self.severity, + confidence=self.confidence, + file=file, + start_line=self.start_line, + end_line=self.end_line, + ) + + +def apply() -> None: + # LLMFinding: drop explanation + remediation, and stop forwarding them. + remove_model_fields(llm_base.LLMFinding, ["explanation", "remediation"]) + llm_base.LLMFinding.to_finding = _pruned_to_finding + llm_base.LLMFinding.model_rebuild(force=True) + llm_base.LLMAnalysisResult.model_rebuild(force=True) + + # MetaAnalyzerFinding: drop intent + impact. + remove_model_fields(meta.MetaAnalyzerFinding, ["intent", "impact"]) + meta.MetaAnalyzerFinding.model_rebuild(force=True) + + # MetaAnalyzerResult: drop overall_assessment field + its validator. + remove_model_fields(meta.MetaAnalyzerResult, ["overall_assessment"]) + pop_field_validator(meta.MetaAnalyzerResult, "_parse_stringified_assessment") + meta.MetaAnalyzerResult.model_rebuild(force=True) diff --git a/tests/exaforce/conftest.py b/tests/exaforce/conftest.py new file mode 100644 index 00000000..6664ab27 --- /dev/null +++ b/tests/exaforce/conftest.py @@ -0,0 +1,21 @@ +import subprocess +import sys +import textwrap + +import pytest + + +@pytest.fixture +def run_in_subprocess(): + """Run *code* in a fresh interpreter; return stdout, assert clean exit.""" + + def _run(code: str) -> str: + result = subprocess.run( + [sys.executable, "-c", textwrap.dedent(code)], + capture_output=True, + text=True, + ) + assert result.returncode == 0, result.stderr + return result.stdout + + return _run diff --git a/tests/exaforce/test_patches.py b/tests/exaforce/test_patches.py new file mode 100644 index 00000000..080dd171 --- /dev/null +++ b/tests/exaforce/test_patches.py @@ -0,0 +1,77 @@ +def test_apply_patches_prunes_model_schemas(run_in_subprocess): + out = run_in_subprocess( + """ + import skillspector + from skillspector.exaforce import apply_patches + apply_patches() + from skillspector.llm_analyzer_base import LLMFinding, LLMAnalysisResult + from skillspector.nodes.meta_analyzer import ( + MetaAnalyzerFinding, + MetaAnalyzerResult, + ) + lf = set(LLMFinding.model_json_schema()["properties"]) + assert "explanation" not in lf and "remediation" not in lf, lf + mf = set(MetaAnalyzerFinding.model_json_schema()["properties"]) + assert "intent" not in mf and "impact" not in mf, mf + mr = set(MetaAnalyzerResult.model_json_schema()["properties"]) + assert "overall_assessment" not in mr, mr + # Container schema (what the LLM actually receives) is pruned too: + nested = LLMAnalysisResult.model_json_schema()["$defs"]["LLMFinding"]["properties"] + assert "explanation" not in nested, nested + print("OK") + """ + ) + assert "OK" in out + + +def test_apply_patches_prunes_to_finding_and_dump(run_in_subprocess): + out = run_in_subprocess( + """ + import skillspector + from skillspector.exaforce import apply_patches + apply_patches() + from skillspector.llm_analyzer_base import LLMFinding + f = LLMFinding(rule_id="R", message="m", severity="LOW", start_line=3) + assert "explanation" not in f.model_dump() + assert "remediation" not in f.model_dump() + fin = f.to_finding("x.py") + assert fin.explanation is None + assert fin.rule_id == "R" and fin.start_line == 3 + print("OK") + """ + ) + assert "OK" in out + + +def test_apply_patches_trims_prompts(run_in_subprocess): + out = run_in_subprocess( + """ + import skillspector + from skillspector.exaforce import apply_patches + apply_patches() + from skillspector.nodes.analyzers import semantic_developer_intent as d + from skillspector.nodes.analyzers import semantic_quality_policy as q + from skillspector.nodes import meta_analyzer as m + assert "Reference the L-prefixed line numbers" not in d.ANALYZER_PROMPT + assert "Reference the L-prefixed line numbers" not in q.ANALYZER_PROMPT + assert "What is the likely intent" not in m.PER_FILE_ANALYSIS_PROMPT + assert "What is the potential impact" not in m.PER_FILE_ANALYSIS_PROMPT + assert "Use the rule IDs exactly as listed." in d.ANALYZER_PROMPT + print("OK") + """ + ) + assert "OK" in out + + +def test_apply_patches_is_idempotent(run_in_subprocess): + out = run_in_subprocess( + """ + import skillspector + from skillspector.exaforce import apply_patches + apply_patches() + apply_patches() + apply_patches() + print("OK") + """ + ) + assert "OK" in out From e6ac14c46fad2690d273eaeb1a6ca2cb2a98bedc Mon Sep 17 00:00:00 2001 From: will-exaforce Date: Fri, 10 Jul 2026 08:20:55 -0500 Subject: [PATCH 09/11] feat(exaforce): activate schema-pruning patch on import Runtime now matches PR #8. The two reverted upstream test files assert the un-pruned schema and fail by design (see docs/superpowers/EXPECTED_TEST_FAILURES.md). --- docs/superpowers/EXPECTED_TEST_FAILURES.md | 22 ++++++++++++++++++++++ src/skillspector/__init__.py | 4 ++++ tests/exaforce/test_activation.py | 19 +++++++++++++++++++ 3 files changed, 45 insertions(+) create mode 100644 docs/superpowers/EXPECTED_TEST_FAILURES.md create mode 100644 tests/exaforce/test_activation.py diff --git a/docs/superpowers/EXPECTED_TEST_FAILURES.md b/docs/superpowers/EXPECTED_TEST_FAILURES.md new file mode 100644 index 00000000..2ddc9cc0 --- /dev/null +++ b/docs/superpowers/EXPECTED_TEST_FAILURES.md @@ -0,0 +1,22 @@ +# Expected test failures (fork: exaforce schema pruning) + +These upstream tests are kept at upstream parity on purpose and therefore +assert the *un-pruned* schema, which the exaforce runtime patch removes. They +are expected to FAIL. A failure here is only a problem if the failure is NOT an +assertion about a pruned key (e.g. an import/collection error). + +Captured from: +`uv run pytest tests/nodes/test_llm_analyzer_base.py tests/nodes/test_semantic_quality_policy.py -q` + +- tests/nodes/test_llm_analyzer_base.py::TestLLMAnalysisResult::test_to_finding +- tests/nodes/test_llm_analyzer_base.py::TestLLMAnalysisResult::test_model_dump +- tests/nodes/test_llm_analyzer_base.py::TestMetaAnalyzerResult::test_intent_validation +- tests/nodes/test_semantic_quality_policy.py::TestFixtureMaliciousSkill::test_malicious_skill_findings_preserve_metadata + +All four fail with an `AssertionError` (or `KeyError`) about a pruned key +(`explanation`, `intent`) being absent — not an import/collection error. +Confirmed bounded to these two files via `uv run pytest -q -rf`: + +``` +4 failed, 1261 passed, 13 skipped, 34 deselected, 6 xfailed +``` diff --git a/src/skillspector/__init__.py b/src/skillspector/__init__.py index 30ce9322..27d7c776 100644 --- a/src/skillspector/__init__.py +++ b/src/skillspector/__init__.py @@ -35,3 +35,7 @@ from skillspector.graph import create_graph, graph # noqa: E402 (after filter setup) __all__ = ["create_graph", "graph", "__version__"] + +# ExaForce fork: apply runtime schema/prompt patches (kept out of upstream files). +from skillspector import exaforce as _exaforce # noqa: E402 +_exaforce.apply_patches() diff --git a/tests/exaforce/test_activation.py b/tests/exaforce/test_activation.py new file mode 100644 index 00000000..88c5e130 --- /dev/null +++ b/tests/exaforce/test_activation.py @@ -0,0 +1,19 @@ +def test_bare_import_activates_patches(run_in_subprocess): + # No explicit apply_patches() call — importing skillspector must patch. + out = run_in_subprocess( + """ + import skillspector + from skillspector.llm_analyzer_base import LLMFinding + from skillspector.nodes.meta_analyzer import ( + MetaAnalyzerFinding, + MetaAnalyzerResult, + ) + from skillspector.nodes import meta_analyzer as m + assert "explanation" not in LLMFinding.model_json_schema()["properties"] + assert "intent" not in MetaAnalyzerFinding.model_json_schema()["properties"] + assert "overall_assessment" not in MetaAnalyzerResult.model_json_schema()["properties"] + assert "What is the likely intent" not in m.PER_FILE_ANALYSIS_PROMPT + print("OK") + """ + ) + assert "OK" in out From cb631d8455c48afbc5463430e4206188c246a19d Mon Sep 17 00:00:00 2001 From: will-exaforce Date: Fri, 10 Jul 2026 08:28:54 -0500 Subject: [PATCH 10/11] test(exaforce): assert meta-analyzer nested pruning and remediation drop --- tests/exaforce/test_patches.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/exaforce/test_patches.py b/tests/exaforce/test_patches.py index 080dd171..e089e84e 100644 --- a/tests/exaforce/test_patches.py +++ b/tests/exaforce/test_patches.py @@ -18,6 +18,8 @@ def test_apply_patches_prunes_model_schemas(run_in_subprocess): # Container schema (what the LLM actually receives) is pruned too: nested = LLMAnalysisResult.model_json_schema()["$defs"]["LLMFinding"]["properties"] assert "explanation" not in nested, nested + nested_meta = MetaAnalyzerResult.model_json_schema()["$defs"]["MetaAnalyzerFinding"]["properties"] + assert "intent" not in nested_meta and "impact" not in nested_meta, nested_meta print("OK") """ ) @@ -36,6 +38,7 @@ def test_apply_patches_prunes_to_finding_and_dump(run_in_subprocess): assert "remediation" not in f.model_dump() fin = f.to_finding("x.py") assert fin.explanation is None + assert fin.remediation is None assert fin.rule_id == "R" and fin.start_line == 3 print("OK") """ From 5c17e2a147296c0019d564f9edce60d4a0532a9c Mon Sep 17 00:00:00 2001 From: will-exaforce Date: Fri, 10 Jul 2026 09:23:57 -0500 Subject: [PATCH 11/11] chore: drop superpowers plan/spec artifacts from PR These were process/design docs, not product deliverables. The functional EXPECTED_TEST_FAILURES.md is kept (referenced by the PR description). --- ...-10-exaforce-monkeypatch-schema-pruning.md | 649 ------------------ ...07-10-monkeypatch-schema-pruning-design.md | 153 ----- 2 files changed, 802 deletions(-) delete mode 100644 docs/superpowers/plans/2026-07-10-exaforce-monkeypatch-schema-pruning.md delete mode 100644 docs/superpowers/specs/2026-07-10-monkeypatch-schema-pruning-design.md diff --git a/docs/superpowers/plans/2026-07-10-exaforce-monkeypatch-schema-pruning.md b/docs/superpowers/plans/2026-07-10-exaforce-monkeypatch-schema-pruning.md deleted file mode 100644 index 8d594666..00000000 --- a/docs/superpowers/plans/2026-07-10-exaforce-monkeypatch-schema-pruning.md +++ /dev/null @@ -1,649 +0,0 @@ -# ExaForce Monkeypatch Schema-Pruning Implementation Plan - -> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. - -**Goal:** Reproduce PR #8's LLM schema/prompt pruning at runtime from the fork-only `src/skillspector/exaforce/` package, and restore every upstream-tracked file it edited to upstream parity. - -**Architecture:** A guarded monkeypatch. On `import skillspector`, `exaforce.apply_patches()` mutates the existing Pydantic model classes in place (`del model_fields[...]` + `model_rebuild(force=True)` on leaf and container, preserving class identity) and rewrites three prompt module-globals via targeted `str.replace()`. Every mutation asserts its upstream target still exists and raises `PatchDriftError` if not, so an upstream rename fails loudly instead of silently going stale. - -**Tech Stack:** Python 3.x, Pydantic v2, pytest, `uv` for running everything. - -## Global Constraints - -- Only `src/skillspector/exaforce/**` and **one** appended block in `src/skillspector/__init__.py` may diverge from upstream. Every other upstream-tracked file (source and tests) must end at upstream (pre-PR-#8) parity. Pre-PR-#8 tree = commit `bec96da` (`326848f^`). -- All guards must **raise** (`PatchDriftError`) on drift — never silently no-op a mutation. -- `apply_patches()` must be idempotent (safe to call repeatedly). -- Do **not** touch unrelated working-tree changes already present (`benchmark/**`, `.claude/**`, `.superpowers/**`, `docs/superpowers/**`, `*.db`, `*_report.md`, etc.). Stage only the exact files each task names. -- Run all commands via `uv run`. Test suite command: `uv run pytest`. -- The scanned skill content is adversarial input; do not change any security invariant — only reproduce PR #8's key/prompt pruning. - ---- - -### Task 1: Revert PR #8 files to upstream parity - -Undo PR #8's edits so the four source files are un-pruned again (the patch in later tasks needs those fields/prompts present to remove), and the two test files return to upstream. After this task the tree equals pre-PR-#8 and the suite is green. - -**Files:** -- Modify (revert): `src/skillspector/llm_analyzer_base.py` -- Modify (revert): `src/skillspector/nodes/meta_analyzer.py` -- Modify (revert): `src/skillspector/nodes/analyzers/semantic_developer_intent.py` -- Modify (revert): `src/skillspector/nodes/analyzers/semantic_quality_policy.py` -- Modify (revert): `tests/nodes/test_llm_analyzer_base.py` -- Modify (revert): `tests/nodes/test_semantic_quality_policy.py` - -**Interfaces:** -- Produces (restored upstream symbols later tasks patch): `llm_analyzer_base.LLMFinding` (with fields `explanation`, `remediation` and a `to_finding` that forwards them), `llm_analyzer_base.LLMAnalysisResult`; `meta_analyzer.MetaAnalyzerFinding` (with `intent`, `impact`), `meta_analyzer.MetaAnalyzerResult` (with `overall_assessment` field + `_parse_stringified_assessment` field_validator), `meta_analyzer.PER_FILE_ANALYSIS_PROMPT`; `semantic_developer_intent.ANALYZER_PROMPT`; `semantic_quality_policy.ANALYZER_PROMPT`. - -- [ ] **Step 1: Restore the six files from the pre-PR-#8 commit** - -```bash -git checkout 326848f^ -- \ - src/skillspector/llm_analyzer_base.py \ - src/skillspector/nodes/meta_analyzer.py \ - src/skillspector/nodes/analyzers/semantic_developer_intent.py \ - src/skillspector/nodes/analyzers/semantic_quality_policy.py \ - tests/nodes/test_llm_analyzer_base.py \ - tests/nodes/test_semantic_quality_policy.py -``` - -- [ ] **Step 2: Verify the four source files now contain the upstream (un-pruned) symbols** - -Run: -```bash -git diff 326848f^ -- \ - src/skillspector/llm_analyzer_base.py \ - src/skillspector/nodes/meta_analyzer.py \ - src/skillspector/nodes/analyzers/semantic_developer_intent.py \ - src/skillspector/nodes/analyzers/semantic_quality_policy.py \ - tests/nodes/test_llm_analyzer_base.py \ - tests/nodes/test_semantic_quality_policy.py -``` -Expected: **no output** (working tree matches pre-PR-#8 for these files). - -Also confirm the fields are back: -```bash -grep -n "explanation\|remediation" src/skillspector/llm_analyzer_base.py -grep -n "intent\|impact\|overall_assessment\|OverallAssessment" src/skillspector/nodes/meta_analyzer.py -``` -Expected: matches present (fields restored). - -- [ ] **Step 3: Run the suite to confirm the reverted baseline is green** - -Run: `uv run pytest -q` -Expected: PASS (source and tests are both un-pruned, so consistent). Note the passing count for comparison in Task 4. - -- [ ] **Step 4: Commit** - -```bash -git add \ - src/skillspector/llm_analyzer_base.py \ - src/skillspector/nodes/meta_analyzer.py \ - src/skillspector/nodes/analyzers/semantic_developer_intent.py \ - src/skillspector/nodes/analyzers/semantic_quality_policy.py \ - tests/nodes/test_llm_analyzer_base.py \ - tests/nodes/test_semantic_quality_policy.py -git commit -m "revert: restore PR #8 files to upstream parity - -Pruning moves to the fork-only exaforce runtime patch (subsequent tasks)." -``` - ---- - -### Task 2: exaforce guard primitives + unit tests - -Add the small, dependency-free patch primitives used by every later patch, with the drift guards. No real models are touched yet — these are tested against throwaway models. - -**Files:** -- Create: `src/skillspector/exaforce/_patchlib.py` -- Create: `tests/exaforce/__init__.py` (empty) -- Test: `tests/exaforce/test_patchlib.py` - -Note: `src/skillspector/exaforce/__init__.py` already exists (empty); leave it for Task 3. - -**Interfaces:** -- Produces: - - `class PatchDriftError(RuntimeError)` - - `remove_model_fields(model: type[BaseModel], field_names: list[str]) -> None` — deletes each name from `model.model_fields`; raises `PatchDriftError` if a name is absent. Caller must `model_rebuild(force=True)` afterward. - - `pop_field_validator(model: type[BaseModel], validator_name: str) -> None` — removes a field_validator decorator by function name; no-op if absent. - - `replace_module_str(module: ModuleType, attr: str, old: str, new: str) -> None` — sets `module.attr = module.attr.replace(old, new)`; raises `PatchDriftError` if `old` not present. - -- [ ] **Step 1: Write the failing tests** - -Create `tests/exaforce/__init__.py` (empty file), then `tests/exaforce/test_patchlib.py`: - -```python -from types import ModuleType - -import pytest -from pydantic import BaseModel - -from skillspector.exaforce._patchlib import ( - PatchDriftError, - pop_field_validator, - remove_model_fields, - replace_module_str, -) - - -def test_remove_model_fields_removes_then_rebuild_drops_key(): - class M(BaseModel): - a: str - b: str = "" - - remove_model_fields(M, ["b"]) - M.model_rebuild(force=True) - assert "b" not in M.model_json_schema()["properties"] - assert "a" in M.model_json_schema()["properties"] - - -def test_remove_model_fields_raises_on_missing_field(): - class M(BaseModel): - a: str - - with pytest.raises(PatchDriftError): - remove_model_fields(M, ["nope"]) - - -def test_pop_field_validator_is_noop_when_absent(): - class M(BaseModel): - a: str - - pop_field_validator(M, "nonexistent") # must not raise - - -def test_replace_module_str_replaces_substring(): - mod = ModuleType("dummy") - mod.P = "hello world" - replace_module_str(mod, "P", "world", "there") - assert mod.P == "hello there" - - -def test_replace_module_str_raises_on_missing_substring(): - mod = ModuleType("dummy") - mod.P = "hello world" - with pytest.raises(PatchDriftError): - replace_module_str(mod, "P", "absent", "x") -``` - -- [ ] **Step 2: Run the tests to verify they fail** - -Run: `uv run pytest tests/exaforce/test_patchlib.py -q` -Expected: FAIL with `ModuleNotFoundError`/`ImportError` (`_patchlib` does not exist yet). - -- [ ] **Step 3: Implement `_patchlib.py`** - -Create `src/skillspector/exaforce/_patchlib.py`: - -```python -# SPDX-License-Identifier: Apache-2.0 -"""Guarded monkeypatch primitives for the ExaForce fork. - -Each primitive asserts its upstream target still exists before mutating it, so -an upstream rename/rewrite fails loudly (``PatchDriftError``) at import time -instead of silently leaving the fork's runtime behavior stale. -""" - -from __future__ import annotations - -from types import ModuleType - -from pydantic import BaseModel - - -class PatchDriftError(RuntimeError): - """An upstream target a fork patch depends on has changed or is missing.""" - - -def remove_model_fields(model: type[BaseModel], field_names: list[str]) -> None: - """Delete ``field_names`` from ``model.model_fields``. - - The caller must run ``model.model_rebuild(force=True)`` afterward (and on any - container model that nests ``model``) for the change to reach the emitted - JSON schema. Raises ``PatchDriftError`` if a field is absent. - """ - for name in field_names: - if name not in model.model_fields: - raise PatchDriftError( - f"{model.__module__}.{model.__qualname__} has no field {name!r}; " - "upstream changed — update the exaforce patch." - ) - del model.model_fields[name] - - -def pop_field_validator(model: type[BaseModel], validator_name: str) -> None: - """Remove a ``field_validator`` decorator by its function name (no-op if absent).""" - model.__pydantic_decorators__.field_validators.pop(validator_name, None) - - -def replace_module_str(module: ModuleType, attr: str, old: str, new: str) -> None: - """Replace substring ``old`` with ``new`` in module-global ``attr``. - - Raises ``PatchDriftError`` if ``old`` is not present in the current value. - """ - current = getattr(module, attr) - if old not in current: - raise PatchDriftError( - f"{module.__name__}.{attr} does not contain the expected text; " - "upstream changed — update the exaforce patch." - ) - setattr(module, attr, current.replace(old, new)) -``` - -- [ ] **Step 4: Run the tests to verify they pass** - -Run: `uv run pytest tests/exaforce/test_patchlib.py -q` -Expected: PASS (5 passed). - -- [ ] **Step 5: Commit** - -```bash -git add src/skillspector/exaforce/_patchlib.py tests/exaforce/__init__.py tests/exaforce/test_patchlib.py -git commit -m "feat(exaforce): guarded monkeypatch primitives" -``` - ---- - -### Task 3: Schema + prompt patches and `apply_patches()` - -Implement the actual pruning patches and the idempotent `apply_patches()` orchestrator. **No activation from `skillspector/__init__.py` yet** — this task tests via a subprocess that imports skillspector and calls `apply_patches()` explicitly, so the in-process suite (and the reverted upstream tests) stay green. - -**Files:** -- Create: `src/skillspector/exaforce/_schema_patches.py` -- Create: `src/skillspector/exaforce/_prompt_patches.py` -- Modify: `src/skillspector/exaforce/__init__.py` (currently empty) -- Create: `tests/exaforce/conftest.py` (shared `run_in_subprocess` fixture, reused by Task 4) -- Test: `tests/exaforce/test_patches.py` - -**Interfaces:** -- Consumes: `PatchDriftError`, `remove_model_fields`, `pop_field_validator`, `replace_module_str` from `_patchlib` (Task 2); the restored upstream symbols from Task 1. -- Produces: - - `skillspector.exaforce.apply_patches() -> None` — idempotent; runs `_schema_patches.apply()` then `_prompt_patches.apply()`. - - `_schema_patches.apply() -> None`, `_prompt_patches.apply() -> None`. - -- [ ] **Step 1: Write the shared fixture and the failing tests** - -Create `tests/exaforce/conftest.py` (the subprocess runner both this task and Task 4 use — a fresh interpreter is how we assert the *real* import-time behavior without polluting the in-process patch state): - -```python -import subprocess -import sys -import textwrap - -import pytest - - -@pytest.fixture -def run_in_subprocess(): - """Run *code* in a fresh interpreter; return stdout, assert clean exit.""" - - def _run(code: str) -> str: - result = subprocess.run( - [sys.executable, "-c", textwrap.dedent(code)], - capture_output=True, - text=True, - ) - assert result.returncode == 0, result.stderr - return result.stdout - - return _run -``` - -Then create `tests/exaforce/test_patches.py`: - -```python -def test_apply_patches_prunes_model_schemas(run_in_subprocess): - out = run_in_subprocess( - """ - import skillspector - from skillspector.exaforce import apply_patches - apply_patches() - from skillspector.llm_analyzer_base import LLMFinding, LLMAnalysisResult - from skillspector.nodes.meta_analyzer import ( - MetaAnalyzerFinding, - MetaAnalyzerResult, - ) - lf = set(LLMFinding.model_json_schema()["properties"]) - assert "explanation" not in lf and "remediation" not in lf, lf - mf = set(MetaAnalyzerFinding.model_json_schema()["properties"]) - assert "intent" not in mf and "impact" not in mf, mf - mr = set(MetaAnalyzerResult.model_json_schema()["properties"]) - assert "overall_assessment" not in mr, mr - # Container schema (what the LLM actually receives) is pruned too: - nested = LLMAnalysisResult.model_json_schema()["$defs"]["LLMFinding"]["properties"] - assert "explanation" not in nested, nested - print("OK") - """ - ) - assert "OK" in out - - -def test_apply_patches_prunes_to_finding_and_dump(run_in_subprocess): - out = run_in_subprocess( - """ - import skillspector - from skillspector.exaforce import apply_patches - apply_patches() - from skillspector.llm_analyzer_base import LLMFinding - f = LLMFinding(rule_id="R", message="m", severity="LOW", start_line=3) - assert "explanation" not in f.model_dump() - assert "remediation" not in f.model_dump() - fin = f.to_finding("x.py") - assert fin.explanation is None - assert fin.rule_id == "R" and fin.start_line == 3 - print("OK") - """ - ) - assert "OK" in out - - -def test_apply_patches_trims_prompts(run_in_subprocess): - out = run_in_subprocess( - """ - import skillspector - from skillspector.exaforce import apply_patches - apply_patches() - from skillspector.nodes.analyzers import semantic_developer_intent as d - from skillspector.nodes.analyzers import semantic_quality_policy as q - from skillspector.nodes import meta_analyzer as m - assert "Reference the L-prefixed line numbers" not in d.ANALYZER_PROMPT - assert "Reference the L-prefixed line numbers" not in q.ANALYZER_PROMPT - assert "What is the likely intent" not in m.PER_FILE_ANALYSIS_PROMPT - assert "What is the potential impact" not in m.PER_FILE_ANALYSIS_PROMPT - assert "Use the rule IDs exactly as listed." in d.ANALYZER_PROMPT - print("OK") - """ - ) - assert "OK" in out - - -def test_apply_patches_is_idempotent(run_in_subprocess): - out = run_in_subprocess( - """ - import skillspector - from skillspector.exaforce import apply_patches - apply_patches() - apply_patches() - apply_patches() - print("OK") - """ - ) - assert "OK" in out -``` - -- [ ] **Step 2: Run the tests to verify they fail** - -Run: `uv run pytest tests/exaforce/test_patches.py -q` -Expected: FAIL — subprocess exits non-zero because `apply_patches` is not importable yet (empty `exaforce/__init__.py`), so `_run` hits its `assert result.returncode == 0`. - -- [ ] **Step 3: Implement `_schema_patches.py`** - -Create `src/skillspector/exaforce/_schema_patches.py`: - -```python -# SPDX-License-Identifier: Apache-2.0 -"""Prune unused keys from the LLM structured-output schemas (fork behavior). - -Reproduces the schema pruning that would otherwise live in upstream -``llm_analyzer_base`` / ``meta_analyzer``, keeping those upstream files at -parity with NVIDIA/SkillSpector. -""" - -from __future__ import annotations - -import skillspector.llm_analyzer_base as llm_base -import skillspector.nodes.meta_analyzer as meta -from skillspector.models import Finding - -from ._patchlib import pop_field_validator, remove_model_fields - - -def _pruned_to_finding(self: "llm_base.LLMFinding", file: str) -> Finding: - """``LLMFinding.to_finding`` without the removed explanation/remediation.""" - return Finding( - rule_id=self.rule_id, - message=self.message, - severity=self.severity, - confidence=self.confidence, - file=file, - start_line=self.start_line, - end_line=self.end_line, - ) - - -def apply() -> None: - # LLMFinding: drop explanation + remediation, and stop forwarding them. - remove_model_fields(llm_base.LLMFinding, ["explanation", "remediation"]) - llm_base.LLMFinding.to_finding = _pruned_to_finding - llm_base.LLMFinding.model_rebuild(force=True) - llm_base.LLMAnalysisResult.model_rebuild(force=True) - - # MetaAnalyzerFinding: drop intent + impact. - remove_model_fields(meta.MetaAnalyzerFinding, ["intent", "impact"]) - meta.MetaAnalyzerFinding.model_rebuild(force=True) - - # MetaAnalyzerResult: drop overall_assessment field + its validator. - remove_model_fields(meta.MetaAnalyzerResult, ["overall_assessment"]) - pop_field_validator(meta.MetaAnalyzerResult, "_parse_stringified_assessment") - meta.MetaAnalyzerResult.model_rebuild(force=True) -``` - -- [ ] **Step 4: Implement `_prompt_patches.py`** - -Create `src/skillspector/exaforce/_prompt_patches.py`: - -```python -# SPDX-License-Identifier: Apache-2.0 -"""Trim prompt text the pruned schema no longer needs (fork behavior).""" - -from __future__ import annotations - -import skillspector.nodes.analyzers.semantic_developer_intent as dev_intent -import skillspector.nodes.analyzers.semantic_quality_policy as quality -import skillspector.nodes.meta_analyzer as meta - -from ._patchlib import replace_module_str - -# Present verbatim in both semantic analyzers' ANALYZER_PROMPT (note the two -# spaces after "listed." and the mid-sentence newline). -_LINE_NUMBER_OLD = ( - "Use the rule IDs exactly as listed. Reference the L-prefixed line numbers\n" - "when reporting findings." -) -_LINE_NUMBER_NEW = "Use the rule IDs exactly as listed." - -# The meta-analyzer's "Your Task" list: drop the intent (2) and impact (3) items. -_META_TASK_OLD = ( - "1. Is this a true vulnerability or a false positive?\n" - "2. What is the likely intent (malicious, negligent, or benign)?\n" - "3. What is the potential impact if exploited?\n" - "4. Does the skill context make this more or less dangerous?" -) -_META_TASK_NEW = ( - "1. Is this a true vulnerability or a false positive?\n" - "2. Does the skill context make this more or less dangerous?" -) - - -def apply() -> None: - replace_module_str(dev_intent, "ANALYZER_PROMPT", _LINE_NUMBER_OLD, _LINE_NUMBER_NEW) - replace_module_str(quality, "ANALYZER_PROMPT", _LINE_NUMBER_OLD, _LINE_NUMBER_NEW) - replace_module_str(meta, "PER_FILE_ANALYSIS_PROMPT", _META_TASK_OLD, _META_TASK_NEW) -``` - -- [ ] **Step 5: Implement `exaforce/__init__.py`** - -Overwrite `src/skillspector/exaforce/__init__.py`: - -```python -# SPDX-License-Identifier: Apache-2.0 -"""ExaForce fork-local runtime patches. - -Keeps fork behavior — pruning unused LLM structured-output keys and prompt text -to shrink requests and reduce LLM timeouts — out of upstream-tracked source -files. All mutations are guarded: an upstream rename/rewrite raises -``PatchDriftError`` at import time rather than silently going stale. -""" - -from __future__ import annotations - -from . import _prompt_patches, _schema_patches - -_PATCHED = False - - -def apply_patches() -> None: - """Idempotently apply all fork-local runtime patches.""" - global _PATCHED - if _PATCHED: - return - _schema_patches.apply() - _prompt_patches.apply() - _PATCHED = True -``` - -- [ ] **Step 6: Run the tests to verify they pass** - -Run: `uv run pytest tests/exaforce/test_patches.py -q` -Expected: PASS (4 passed). - -- [ ] **Step 7: Confirm the in-process suite is still green (no activation yet)** - -Run: `uv run pytest -q` -Expected: PASS — same count as Task 1 Step 3 plus the new exaforce tests. The reverted upstream tests still pass because `apply_patches()` has not been wired into `import skillspector` yet (it only runs inside the subprocess tests). - -- [ ] **Step 8: Commit** - -```bash -git add src/skillspector/exaforce/_schema_patches.py \ - src/skillspector/exaforce/_prompt_patches.py \ - src/skillspector/exaforce/__init__.py \ - tests/exaforce/conftest.py \ - tests/exaforce/test_patches.py -git commit -m "feat(exaforce): prune LLM schema keys + prompt text via guarded patch" -``` - ---- - -### Task 4: Activate on import + document expected upstream-test failures - -Wire `apply_patches()` into `skillspector/__init__.py` so the pruning is the default everywhere (CLI, MCP, benchmark, tests). This makes the runtime match PR #8 exactly. As an accepted consequence, the two reverted upstream test files now fail (they assert the un-pruned schema); capture that exact set. - -**Files:** -- Modify: `src/skillspector/__init__.py` (append activation block after `__all__`) -- Create: `tests/exaforce/test_activation.py` -- Create: `docs/superpowers/EXPECTED_TEST_FAILURES.md` - -**Interfaces:** -- Consumes: `skillspector.exaforce.apply_patches` (Task 3). -- Produces: bare `import skillspector` yields pruned schemas/prompts. - -- [ ] **Step 1: Write the failing activation test** - -Create `tests/exaforce/test_activation.py` (reuses the `run_in_subprocess` fixture from `tests/exaforce/conftest.py`, added in Task 3): - -```python -def test_bare_import_activates_patches(run_in_subprocess): - # No explicit apply_patches() call — importing skillspector must patch. - out = run_in_subprocess( - """ - import skillspector - from skillspector.llm_analyzer_base import LLMFinding - from skillspector.nodes.meta_analyzer import ( - MetaAnalyzerFinding, - MetaAnalyzerResult, - ) - from skillspector.nodes import meta_analyzer as m - assert "explanation" not in LLMFinding.model_json_schema()["properties"] - assert "intent" not in MetaAnalyzerFinding.model_json_schema()["properties"] - assert "overall_assessment" not in MetaAnalyzerResult.model_json_schema()["properties"] - assert "What is the likely intent" not in m.PER_FILE_ANALYSIS_PROMPT - print("OK") - """ - ) - assert "OK" in out -``` - -- [ ] **Step 2: Run it to verify it fails** - -Run: `uv run pytest tests/exaforce/test_activation.py -q` -Expected: FAIL — the subprocess assertion trips (bare import does not patch yet), so `_run` sees a non-zero exit. - -- [ ] **Step 3: Append the activation block to `skillspector/__init__.py`** - -Add these two lines at the **end** of `src/skillspector/__init__.py` (after the existing `__all__ = [...]` line — by this point `from skillspector.graph import ...` has already imported the target modules): - -```python - -# ExaForce fork: apply runtime schema/prompt patches (kept out of upstream files). -from skillspector import exaforce as _exaforce # noqa: E402 -_exaforce.apply_patches() -``` - -- [ ] **Step 4: Run the activation test to verify it passes** - -Run: `uv run pytest tests/exaforce/test_activation.py -q` -Expected: PASS (1 passed). - -- [ ] **Step 5: Capture the expected upstream-test failures** - -Run (the `- true` keeps going despite pytest's non-zero exit on failures): -```bash -uv run pytest tests/nodes/test_llm_analyzer_base.py tests/nodes/test_semantic_quality_policy.py -q || true -``` -Expected: some FAILED tests in exactly these two files (assertions expecting `explanation`/`remediation`/`intent`/`impact`/`overall_assessment`). Confirm each failure is an **assertion** about a pruned key — **not** an import/collection error (which would indicate a real bug). - -- [ ] **Step 6: Record the failure set** - -Create `docs/superpowers/EXPECTED_TEST_FAILURES.md` listing the failing node IDs from Step 5, e.g.: - -```markdown -# Expected test failures (fork: exaforce schema pruning) - -These upstream tests are kept at upstream parity on purpose and therefore -assert the *un-pruned* schema, which the exaforce runtime patch removes. They -are expected to FAIL. A failure here is only a problem if the failure is NOT an -assertion about a pruned key (e.g. an import/collection error). - - -- tests/nodes/test_llm_analyzer_base.py::... -- tests/nodes/test_semantic_quality_policy.py::... -``` - -- [ ] **Step 7: Confirm the failure set is bounded to those two files** - -Run: `uv run pytest -q -rf || true` -Expected: the only FAILED tests are inside `tests/nodes/test_llm_analyzer_base.py` and `tests/nodes/test_semantic_quality_policy.py`. The `tests/exaforce/**` suite passes. If any *other* file fails, investigate before finishing (it means the monkeypatch changed behavior beyond PR #8's scope). - -- [ ] **Step 8: Confirm upstream-file parity (the whole point of the fork)** - -Run: -```bash -git diff 326848f^ -- \ - src/skillspector/llm_analyzer_base.py \ - src/skillspector/nodes/meta_analyzer.py \ - src/skillspector/nodes/analyzers/semantic_developer_intent.py \ - src/skillspector/nodes/analyzers/semantic_quality_policy.py \ - tests/nodes/test_llm_analyzer_base.py \ - tests/nodes/test_semantic_quality_policy.py -``` -Expected: **no output**. And `git diff 326848f^ -- src/skillspector/__init__.py` shows only the two-line activation block added. - -- [ ] **Step 9: Commit** - -```bash -git add src/skillspector/__init__.py tests/exaforce/test_activation.py docs/superpowers/EXPECTED_TEST_FAILURES.md -git commit -m "feat(exaforce): activate schema-pruning patch on import - -Runtime now matches PR #8. The two reverted upstream test files assert the -un-pruned schema and fail by design (see docs/superpowers/EXPECTED_TEST_FAILURES.md)." -``` - ---- - -## Post-plan verification - -- [ ] `uv run pytest tests/exaforce -q` → all pass. -- [ ] `uv run pytest -q -rf` → the only failures are in the two reverted upstream test files, matching `docs/superpowers/EXPECTED_TEST_FAILURES.md`. -- [ ] Behavioral equivalence to PR #8: in a fresh interpreter, `import skillspector` then dumping `LLMAnalysisResult.model_json_schema()` and `MetaAnalyzerResult.model_json_schema()` shows the same pruned key set PR #8 produced, and the three prompts lack the removed sentences. -- [ ] `git diff 326848f^` touches only `src/skillspector/exaforce/**`, the two-line block in `src/skillspector/__init__.py`, `tests/exaforce/**`, and `docs/superpowers/**` — no other upstream-tracked file diverges. diff --git a/docs/superpowers/specs/2026-07-10-monkeypatch-schema-pruning-design.md b/docs/superpowers/specs/2026-07-10-monkeypatch-schema-pruning-design.md deleted file mode 100644 index 07823c00..00000000 --- a/docs/superpowers/specs/2026-07-10-monkeypatch-schema-pruning-design.md +++ /dev/null @@ -1,153 +0,0 @@ -# Design: Move PR #8 schema-pruning into an isolated `exaforce` monkeypatch - -- **Date:** 2026-07-10 -- **Branch:** `prune-unused-keys-in-prompt-schemas` (PR #8) -- **Author:** wbeasley - -## Goal - -Reproduce the exact runtime behavior of PR #8 — pruning rarely-used keys out of -the LLM structured-output schemas and prompts to shrink requests and reduce LLM -timeouts — **without editing upstream source files**. The forked change must live -in the fork-only `src/skillspector/exaforce/` package so upstream syncs -(`NVIDIA/SkillSpector`) merge cleanly. - -## What PR #8 changes (the behavior to reproduce) - -| Upstream file | Runtime effect to reproduce | -|---|---| -| `llm_analyzer_base.py` | Remove `explanation`, `remediation` fields from `LLMFinding`; stop passing them in `LLMFinding.to_finding()` | -| `nodes/meta_analyzer.py` | Remove `intent`, `impact` fields from `MetaAnalyzerFinding`; remove `overall_assessment` field + its `_parse_stringified_assessment` validator from `MetaAnalyzerResult` (the `OverallAssessment` class becomes unused/unreferenced); trim the `PER_FILE_ANALYSIS_PROMPT` task list from 4 items to 2 (drop the intent + impact questions) | -| `nodes/analyzers/semantic_developer_intent.py` | Remove the sentence `"Reference the L-prefixed line numbers when reporting findings."` from `ANALYZER_PROMPT` | -| `nodes/analyzers/semantic_quality_policy.py` | Same sentence removal from `ANALYZER_PROMPT` | - -## Approach: in-place monkeypatch, guarded, activated once at import - -### Why in-place mutation (not class replacement) - -Mutating the existing model classes in place preserves object identity, so: -- `LLMAnalyzerBase.response_schema` / `LLMMetaAnalyzer.response_schema` keep - pointing at the same (now-pruned) class objects — no rebinding needed. -- `isinstance(response, LLMAnalysisResult)` in `parse_response` still holds. -- Nothing that imported these classes earlier ends up with a stale reference. - -Verified behavior of the mutation recipe (Pydantic v2): -1. `del Model.model_fields[name]` for each removed field. -2. Pop any dangling `field_validator` decorator for a removed field via - `Model.__pydantic_decorators__.field_validators.pop(, None)`. -3. `Model.model_rebuild(force=True)` on the leaf model **and** on every container - that nests it (e.g. rebuild `LLMFinding` then `LLMAnalysisResult`). - -Confirmed: after this, removed keys disappear from `model_json_schema()` -(including nested `$defs`), from validation, and from `model_dump()`. - -### Prompts - -Prompt bodies are read from module globals at `node()` / analyzer `__init__` -time, so reassigning the module global before those run takes effect. Use a -**guarded targeted `str.replace()`** (assert the target substring is present, -then replace) rather than copying the full prompt text — this avoids duplicating -~100 lines and detects upstream drift. - -### Guards (the key safety property) - -A source-file fork *conflicts loudly* on upstream sync; a monkeypatch instead -*silently goes stale* if upstream renames a field or rewrites a prompt. To -recover the "loud failure" signal: - -- Before removing a field, assert it is present in `model_fields`. -- Before replacing a prompt substring, assert the substring is present. -- On any failed assertion, raise a clear error naming the drifted symbol. - -This turns silent divergence into a fail-fast at process startup. - -### Idempotency - -`apply_patches()` is guarded by a module-level `_PATCHED` flag. The first call -performs all mutations (and runs the drift guards); subsequent calls are no-ops. -This makes repeated imports / explicit test calls safe. - -## Module layout - -``` -src/skillspector/exaforce/ - __init__.py # exposes apply_patches(); holds _PATCHED flag - _schema_patches.py # LLMFinding / MetaAnalyzerFinding / MetaAnalyzerResult field pruning + to_finding - _prompt_patches.py # the three prompt-string edits -``` -(Final file split can collapse to fewer files if trivial; boundary is -schema-patches vs prompt-patches.) - -## Activation - -Append to the end of `src/skillspector/__init__.py` (after the existing -`from skillspector.graph import ...` at line 35, so target modules are loaded): - -```python -from skillspector import exaforce as _exaforce # noqa: E402 -_exaforce.apply_patches() -``` - -This is the **only** upstream-tracked file edited (one idempotent import + -call). It covers CLI, MCP server, the benchmark, and the test suite uniformly, -because any `import skillspector` runs it. - -## Source files: restore to upstream parity - -Revert these 4 files to their pre-PR-#8 (upstream) content, since their pruning -now happens at runtime via `apply_patches()`: -- `src/skillspector/llm_analyzer_base.py` -- `src/skillspector/nodes/meta_analyzer.py` -- `src/skillspector/nodes/analyzers/semantic_developer_intent.py` -- `src/skillspector/nodes/analyzers/semantic_quality_policy.py` - -## Tests - -- **Revert** PR #8's edits to `tests/nodes/test_llm_analyzer_base.py` and - `tests/nodes/test_semantic_quality_policy.py` back to upstream parity. Because - `__init__.py` patches globally on import, these upstream tests assert the - **un-pruned** schema (e.g. `d["explanation"] == ""`, `test_intent_validation` - expecting a `ValueError`) and will therefore **fail** at runtime. This is an - accepted, deliberate trade: keeping every upstream file at parity is worth more - than a green upstream test suite, and avoids the maintenance burden of keeping - forked test assertions in sync with upstream. -- **Add** `tests/exaforce/test_patches.py` (fork-only) that: - - asserts `apply_patches()` removed the expected keys from each model's JSON - schema and from `model_dump()`; - - asserts the three prompt strings no longer contain the removed sentences; - - asserts idempotency (calling `apply_patches()` twice is safe); - - asserts the drift guards raise when a target field/substring is absent - (simulate by monkeypatching a target away, then expecting the guard error). - -## Verification - -- `uv run pytest` passes **except** for the reverted upstream tests that assert - un-pruned behavior (a known, documented set of failures — see Tests). The new - `tests/exaforce/` suite and everything else pass. Record the exact expected - failures so a reviewer can distinguish them from new regressions. -- A one-off runtime check: import `skillspector`, then dump - `LLMAnalysisResult.model_json_schema()` and `MetaAnalyzerResult.model_json_schema()` - and confirm the pruned keys are absent and the prompts lack the removed - sentences — i.e. behavior identical to the PR #8 diff. -- `git diff upstream/main -- src/skillspector/llm_analyzer_base.py src/skillspector/nodes/meta_analyzer.py src/skillspector/nodes/analyzers/semantic_developer_intent.py src/skillspector/nodes/analyzers/semantic_quality_policy.py` - is empty (source files at parity). - -## Out of scope / non-goals - -- No plugin system, no config flag to toggle pruning — pruning is always on - (it is the desired product behavior). -- No reversible/per-test-scoped patching machinery; patches are process-global, - matching the fact that the product always wants the pruned schema. - -## Risks - -1. **Silent drift** if upstream changes a targeted field/prompt — mitigated by - fail-fast guards (raise at startup, don't silently no-op). -2. **Pydantic internal reliance** (`model_fields`, `__pydantic_decorators__`, - `model_rebuild`) could break on a major Pydantic upgrade — mitigated by the - exaforce test suite catching it in CI. -3. **Reverted upstream tests fail at runtime** — accepted. Cost: CI/`pytest` - signal is polluted, since a genuinely new regression in those files would be - harder to spot among the known failures. Mitigated by documenting the exact - expected-failure set. Chosen deliberately over the maintenance burden of - forked test assertions.