diff --git a/.skillsaw.yaml.example b/.skillsaw.yaml.example index 0215bbad..2ef1d24a 100644 --- a/.skillsaw.yaml.example +++ b/.skillsaw.yaml.example @@ -3,6 +3,13 @@ version: "0.18.0" +# Rule-set profile: a curated bundle of severity/enablement overrides applied +# under your own 'rules:' entries. Available: default, claude-5 +# NOTE: the explicit per-rule entries below count as your own overrides and win +# over the profile — to adopt one, also delete the rule entries you want the +# profile to manage. +# profile: claude-5 + rules: # Agent Plugins plugin.json and skills location must conform to 1.0.0 diff --git a/README.md b/README.md index 7b559e2a..f334a4f8 100644 --- a/README.md +++ b/README.md @@ -87,8 +87,10 @@ reference](https://skillsaw.org/rules/) for details. skillsaw works locally, in CI, and inside coding-agent workflows. It provides line-level findings, explanations for every rule, deterministic autofixes, baselines for gradual adoption, GitHub and GitLab integration, and text, JSON, -SARIF, HTML, and Code Climate output. Rules are configurable, and projects can -add local rules or install rule plugins. +SARIF, HTML, and Code Climate output. Rules are configurable — one-line +[profiles](https://skillsaw.org/configuration/#profiles) select curated rule +sets like `claude-5`, tuned to Anthropic's Claude 5 context-engineering +guidance — and projects can add local rules or install rule plugins. | Goal | Documentation | | --- | --- | diff --git a/docs/configuration.md b/docs/configuration.md index 6a7751b7..114f3911 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -70,6 +70,76 @@ automatically but may occasionally fail after a skillsaw upgrade. `version` to your skillsaw version (`skillsaw --version`) and bump it when you upgrade. + The one exception: an unversioned config that selects a curated + [profile](#profiles) follows the **installed** version instead of + falling back to 0.6.0 — new rules apply immediately after upgrades + until you pin `version`. + +## Profiles + +A profile is a named, curated bundle of rule overrides — severities, +enablement, and rule parameters — selected with one config line instead of a +hand-maintained `rules:` block: + +```yaml +version: "0.19.0" +profile: claude-5 +``` + +Profile overrides apply **between** the builtin defaults and your own +`rules:` entries, so precedence is: + +``` +builtin defaults < profile < your rules: overrides +``` + +Anything you set per rule wins over the profile, including an explicit +`enabled: auto` (which restores repo-type/format detection over the +profile's decision). Nested rule parameters (like `context-budget`'s +`limits`) merge per key, so overriding one entry keeps the profile's +others. A severity-only override on a rule the profile +disables does **not** re-enable it — re-enabling takes an explicit +`enabled` setting. `skillsaw explain ` reports when a rule's +enablement or severity comes from the active profile. + +!!! note "Profiles and generated configs" + A config generated by `skillsaw init` lists explicit `enabled` and + `severity` entries for every rule — and those count as *your* + overrides, shadowing most of what a profile would change. To adopt a + profile in a generated config, set `profile:` **and delete** the + per-rule entries you want the profile to manage. + +Available profiles: + +| Profile | Meaning | +|---------|---------| +| `default` | The builtin defaults, unchanged. This is what runs when the key is absent. | +| `claude-5` | Tuned to Anthropic's [Claude 5 context-engineering guidance](https://claude.com/blog/the-new-rules-of-context-engineering-for-claude-5-generation-models). | + +The `claude-5` profile makes these changes over the defaults: + +| Rule | Change | Rationale from the guidance | +|------|--------|------------------------------| +| `content-repeated-directive` | severity → `error` | "Eliminate duplicate instructions" — state each directive once | +| `content-instruction-drift` | severity → `warning` | The same shift, across files: drifted near-duplicates cost budget twice | +| `content-tautological` | severity → `warning` | Remove guidance modern models exhibit by default | +| `content-section-length` | severity → `warning` | Progressive disclosure: split long content into many files | +| `context-budget` | tighter `limits` for `claude-md`/`agents-md`/`gemini-md` (3k warn / 8k error) and `skill` (2k warn / 5k error) | "Keep your CLAUDE.md lightweight" | +| `content-missing-stop-condition` | `enabled: true` | Open-ended loops with no stop condition burn autonomous sessions | +| `content-weak-language` | `enabled: false` | Judgment-delegating prose is the recommended style; flagging hedges fights it | + +A profile's `enabled` decisions bypass the config [`version` +gate](#version-pinning) — profiles ship with the installed skillsaw, so +choosing one is an explicit opt-in to its rule set. Severity- and +parameter-only profile entries never change whether a rule runs; activation +still follows the rule's own default and the `version` gate. + +For the same reason, a config that selects a curated profile but omits +`version` follows the **installed** skillsaw version rather than falling +back to `0.6.0`: new rules apply immediately after upgrades. Set `version` +explicitly to pin the rule set instead. (`profile: default` keeps the +legacy fallback, identical to having no profile key.) + ## Enabling Rules Each rule's `enabled` key accepts three values: diff --git a/src/skillsaw/cli/_explain.py b/src/skillsaw/cli/_explain.py index 032c8050..b26551ee 100644 --- a/src/skillsaw/cli/_explain.py +++ b/src/skillsaw/cli/_explain.py @@ -164,7 +164,15 @@ def _run_explain(args): ) print(f" {state} — {reason}") if enabled: - print(f" severity: {effective_severity}") + # Attribute a profile-supplied severity so the effective value is + # traceable; a user rules: entry overrides the profile and drops + # the annotation. + severity_note = "" + if "severity" in config._profile_rules(args.rule_id) and "severity" not in config.rules.get( + args.rule_id, {} + ): + severity_note = f" (set by profile '{config.profile}')" + print(f" severity: {effective_severity}{severity_note}") if plugin_name is None: # Plugin rules have no page on the skillsaw documentation site. diff --git a/src/skillsaw/config.py b/src/skillsaw/config.py index cab495ce..bbc2d298 100644 --- a/src/skillsaw/config.py +++ b/src/skillsaw/config.py @@ -12,6 +12,7 @@ from typing import Dict, Any, Optional, List, Set, Tuple, TYPE_CHECKING from dataclasses import dataclass, field from skillsaw.paths import safe_resolve +from skillsaw.profiles import DEFAULT_PROFILE, PROFILES, available_profiles if TYPE_CHECKING: from .context import RepositoryContext @@ -76,11 +77,17 @@ class LinterConfig: # Excluded from equality so two configs loaded the same way still compare # equal regardless of the advisory messages attached. warnings: List[str] = field(default_factory=list, compare=False) + # Named rule-set profile (see skillsaw.profiles): a curated bundle of + # rule overrides applied under the user's own ``rules:`` entries. + # Declared after every pre-profile field so the dataclass keeps the + # positional constructor signature of earlier releases. + profile: str = DEFAULT_PROFILE # Recognised top-level config keys; anything else triggers a load warning. _KNOWN_KEYS = frozenset( { "version", + "profile", "rules", "custom-rules", "exclude", @@ -131,15 +138,41 @@ def from_file(cls, config_path: Path) -> "LinterConfig": + ". Known keys: " + ", ".join(sorted(cls._KNOWN_KEYS)) ) + raw_profile = data.get("profile") + if raw_profile is None: + profile = DEFAULT_PROFILE + elif isinstance(raw_profile, str) and raw_profile in PROFILES: + profile = raw_profile + else: + raise ValueError( + f"'profile' must be one of {', '.join(available_profiles())}, " + f"got {raw_profile!r}" + ) + raw_version = data.get("version") + default_version = _DEFAULT_VERSION if raw_version is None: # Covers both a missing key and an explicit ``version:`` (None) — # both would otherwise version-gate as 0.0.0 and disable newer rules. - load_warnings.append( - f"config has no 'version' field; defaulting to {_DEFAULT_VERSION}, so rules " - "added in later versions are silently disabled. Set 'version' to your " - "skillsaw version to enable them." - ) + if profile != DEFAULT_PROFILE: + # Selecting a curated profile is an opt-in to the current + # rule set, so an unpinned config follows the installed + # version: new rules apply immediately after upgrades + # instead of being gated behind a version bump. + from . import __version__ + + default_version = __version__ + load_warnings.append( + f"config has no 'version' field; 'profile: {profile}' implies the " + f"installed version ({__version__}), so new rules apply immediately " + "after upgrades. Set 'version' to pin the rule set instead." + ) + else: + load_warnings.append( + f"config has no 'version' field; defaulting to {_DEFAULT_VERSION}, so rules " + "added in later versions are silently disabled. Set 'version' to your " + "skillsaw version to enable them." + ) raw_rules = data.get("rules") raw_custom_rules = data.get("custom-rules") @@ -286,7 +319,8 @@ def from_file(cls, config_path: Path) -> "LinterConfig": ) return cls( - version=_DEFAULT_VERSION if raw_version is None else str(raw_version), + version=default_version if raw_version is None else str(raw_version), + profile=profile, rules=rules, custom_rules=custom_rules, exclude_patterns=exclude_patterns, @@ -349,10 +383,42 @@ def effective_fail_level(self) -> str: candidates.append("warning") return max(candidates, key=_FAIL_ON_LEVELS.__getitem__) + def _profile_rules(self, rule_id: str) -> Dict[str, Any]: + """Overrides the active profile contributes for *rule_id* ({} if none).""" + profile = PROFILES.get(self.profile) + if profile is None: + return {} + return profile.rules.get(rule_id, {}) + + def _serializable_profile(self) -> bool: + """Whether ``profile`` is worth writing out: a registered, + non-default name. A programmatically-assigned unknown profile + contributes no overrides, so serializing it would produce a file + ``from_file()`` rejects — the effective (default) behavior is + written instead. + """ + return bool(self.profile) and self.profile != DEFAULT_PROFILE and self.profile in PROFILES + + @staticmethod + def _merge_layer(base: Dict[str, Any], overlay: Dict[str, Any]) -> None: + """Merge *overlay* into *base* in place, recursing into mappings. + + Nested dicts merge per key so a layer overriding one entry of a + parameter like ``limits`` keeps the lower layer's other entries; + every non-mapping value (including lists) replaces wholesale. + """ + for key, value in overlay.items(): + if isinstance(value, dict) and isinstance(base.get(key), dict): + LinterConfig._merge_layer(base[key], value) + else: + base[key] = copy.deepcopy(value) + def get_rule_config(self, rule_id: str) -> Dict[str, Any]: """ - Get configuration for a specific rule, merging user overrides - on top of defaults so unmentioned fields keep their default values. + Get configuration for a specific rule, merging the active profile's + overrides and then user overrides on top of defaults, so unmentioned + fields — including entries of nested mappings — keep their default + (or profile-set) values. Args: rule_id: Rule identifier @@ -360,14 +426,15 @@ def get_rule_config(self, rule_id: str) -> Dict[str, Any]: Returns: Rule configuration dict """ - # Deep-copy the cached defaults so callers mutating the merged result - # (or its nested lists like ``recommended-fields``) cannot corrupt the - # shared cache. - defaults = copy.deepcopy(_default_rules().get(rule_id, {})) + # Deep-copy the cached defaults (and, in _merge_layer, every overlay + # value) so callers mutating the merged result — or its nested + # containers like ``limits`` / ``recommended-fields`` — cannot + # corrupt the shared registry caches. + merged = copy.deepcopy(_default_rules().get(rule_id, {})) + self._merge_layer(merged, self._profile_rules(rule_id)) overrides = self.rules.get(rule_id) - if overrides is None: - overrides = {} - merged = {**defaults, **overrides} + if overrides: + self._merge_layer(merged, overrides) return merged def is_rule_enabled( @@ -451,13 +518,35 @@ def rule_enabled_reason( # Deprecated rules never activate through auto detection or default # enablement — only an explicit ``enabled: true`` (handled above) or - # a --rule flag (which bypasses this method) runs them. + # a --rule flag (which bypasses this method) runs them. Checked + # before the profile layer so a profile cannot resurrect one. if deprecated is not None: return False, ( f"deprecated since {deprecated} — will be removed in a future " "release; set 'enabled: true' in config to keep running it" ) + # Profile-set ``enabled`` decides next, under the user's explicit + # setting (handled above) and above everything else. Profiles ship + # with the installed skillsaw, so their decisions bypass the config + # ``version`` gate — choosing one is an explicit opt-in to its rule + # set. That applies to a profile-set ``"auto"`` too: it falls + # through to the detection logic below, but skips the gate (see + # ``profile_sets_enabled``). Severity/parameter-only profile + # entries never change activation. + # ``has_explicit_enabled`` also covers a user ``enabled: "auto"`` — + # any explicit user setting replaces the profile's, so "auto" falls + # through to detection rather than to the profile's decision. + profile_sets_enabled = False + if not has_explicit_enabled: + profile_overrides = self._profile_rules(rule_id) + profile_sets_enabled = "enabled" in profile_overrides + profile_enabled = profile_overrides.get("enabled") + if profile_enabled is True: + return True, f"enabled: true set by profile '{self.profile}'" + if profile_enabled is False: + return False, f"disabled by profile '{self.profile}'" + if not has_explicit_enabled: # Any non-enabled override (e.g. severity) without an explicit # ``enabled`` key on a disabled-by-default rule implies the user @@ -473,10 +562,12 @@ def rule_enabled_reason( # activation — fall through to version gate + auto logic. # Any explicit user override (enabled or otherwise) implies the user - # wants this rule, so skip the version gate. + # wants this rule, so skip the version gate. A profile-set + # ``enabled`` (only "auto" reaches here) skips it the same way — + # the profile's activation decisions are version-independent. has_user_overrides = bool(user_overrides) - if not has_user_overrides and self.version: + if not has_user_overrides and not profile_sets_enabled and self.version: if _parse_version(self.version) < _parse_version(since_version): return False, ( f"config version {self.version} is older than the rule " @@ -514,6 +605,8 @@ def to_dict(self) -> Dict[str, Any]: d: Dict[str, Any] = {} if self.version: d["version"] = self.version + if self._serializable_profile(): + d["profile"] = self.profile d["rules"] = self.rules d["custom-rules"] = self.custom_rules d["exclude"] = self.exclude_patterns @@ -547,6 +640,22 @@ def save(self, config_path: Path): f.write("# https://github.com/stbenjam/skillsaw\n\n") if self.version: f.write(f'version: "{self.version}"\n\n') + f.write( + "# Rule-set profile: a curated bundle of severity/enablement " + "overrides applied\n" + "# under your own 'rules:' entries. Available: " + + ", ".join(available_profiles()) + + "\n" + "# NOTE: the explicit per-rule entries below count as your own " + "overrides and win\n" + "# over the profile — to adopt one, also delete the rule " + "entries you want the\n" + "# profile to manage.\n" + ) + if self._serializable_profile(): + f.write(f"profile: {self._yaml_value(self.profile)}\n\n") + else: + f.write("# profile: claude-5\n\n") f.write("rules:\n") for rule_id, rule_config in self.rules.items(): desc = descriptions.get(rule_id, "") diff --git a/src/skillsaw/profiles.py b/src/skillsaw/profiles.py new file mode 100644 index 00000000..6d796cb3 --- /dev/null +++ b/src/skillsaw/profiles.py @@ -0,0 +1,109 @@ +"""Rule-set profiles: named, curated bundles of rule overrides. + +A profile is a set of per-rule overrides (``enabled``, ``severity``, and +rule parameters) applied between the builtin defaults and the user's own +``rules:`` section, so precedence is: + + builtin defaults < profile < user ``rules:`` overrides + +Selecting one takes a single config line (``profile: claude-5``) instead of +a hand-maintained ``rules:`` block. The ``default`` profile is empty by +definition — it exists so configs can name the current behavior explicitly. + +Profiles are versioned with skillsaw itself: every rule a profile names +ships in the same release, so profile-set ``enabled`` decisions bypass the +config ``version`` gate (choosing a profile is an explicit opt-in to its +rule set). Severity-only and parameter-only entries never change whether a +rule runs — activation still follows the rule's own default and the +``version`` gate. + +Adding a profile: add a :class:`Profile` to :data:`PROFILES` with a +rationale comment per rule entry. Rule IDs must be canonical (no legacy +aliases) and must not name deprecated rules — ``tests/test_config.py`` +(``TestProfileRegistry``) enforces both, plus valid severities and +``enabled`` values, so a bad entry fails fast in CI rather than being +silently ignored at lint time. +""" + +from dataclasses import dataclass, field +from typing import Any, Dict, List + +DEFAULT_PROFILE = "default" + + +@dataclass(frozen=True) +class Profile: + """A named bundle of per-rule config overrides.""" + + name: str + description: str + # rule_id -> overrides merged under the user's ``rules:`` entry for + # that rule. Same shape as a config file rule entry: ``enabled``, + # ``severity``, and any rule parameters from its ``config_schema``. + rules: Dict[str, Dict[str, Any]] = field(default_factory=dict) + + +# The claude-5 profile encodes Anthropic's Claude 5 context-engineering +# guidance ("The new rules of context engineering for Claude 5 generation +# models", https://claude.com/blog/the-new-rules-of-context-engineering-for-claude-5-generation-models) +# as severity and enablement deltas over the defaults. See issue #444 for +# the rule-by-rule mapping of the post's shifts onto skillsaw's rule set. +_CLAUDE_5 = Profile( + name="claude-5", + description=( + "Tuned to Anthropic's Claude 5 context-engineering guidance: " + "elevates duplication and context-bloat rules, tightens token " + "budgets, enables the stop-condition check, and drops " + "hedging-language pedantry" + ), + rules={ + # "Eliminate duplicate instructions" is a named shift in the post; + # a directive stated twice in one file is the clearest violation + # of it. + "content-repeated-directive": {"severity": "error"}, + # The same shift, across files: drifted near-duplicate sections + # cost budget twice and disagree with each other. + "content-instruction-drift": {"severity": "warning"}, + # The post says to remove guidance modern models exhibit by + # default ("be concise", "think step by step"); tautologies are + # pure context waste under a lean-context posture. + "content-tautological": {"severity": "warning"}, + # Progressive disclosure: "for long skills, divide it into many + # files and split them out" — oversized single sections are the + # in-file version of the problem. + "content-section-length": {"severity": "warning"}, + # "Keep your CLAUDE.md lightweight": tighten primary instruction + # files toward the post's bar, and skills toward + # split-and-reference territory. Other categories keep defaults. + "context-budget": { + "limits": { + "claude-md": {"warn": 3000, "error": 8000}, + "agents-md": {"warn": 3000, "error": 8000}, + "gemini-md": {"warn": 3000, "error": 8000}, + "skill": {"warn": 2000, "error": 5000}, + }, + }, + # Opt-in agent-safety rule the profile turns on: open-ended loops + # ("keep monitoring...") with no stop condition burn autonomous + # sessions. + "content-missing-stop-condition": {"enabled": True}, + # Judgment-delegating prose ("use judgment", "when appropriate") + # is the style the post recommends over rigid absolutes; flagging + # hedges fights it. + "content-weak-language": {"enabled": False}, + }, +) + + +PROFILES: Dict[str, Profile] = { + DEFAULT_PROFILE: Profile( + name=DEFAULT_PROFILE, + description="The builtin defaults, unchanged", + ), + _CLAUDE_5.name: _CLAUDE_5, +} + + +def available_profiles() -> List[str]: + """Profile names for error messages and docs, default first.""" + return sorted(PROFILES, key=lambda name: (name != DEFAULT_PROFILE, name)) diff --git a/tests/fixtures/config/profile-claude-5/.skillsaw.yaml b/tests/fixtures/config/profile-claude-5/.skillsaw.yaml new file mode 100644 index 00000000..9c2724a8 --- /dev/null +++ b/tests/fixtures/config/profile-claude-5/.skillsaw.yaml @@ -0,0 +1,2 @@ +version: "99.0.0" +profile: claude-5 diff --git a/tests/fixtures/config/profile-claude-5/CLAUDE.md b/tests/fixtures/config/profile-claude-5/CLAUDE.md new file mode 100644 index 00000000..572f8fb9 --- /dev/null +++ b/tests/fixtures/config/profile-claude-5/CLAUDE.md @@ -0,0 +1,28 @@ +# payments-service + +Go service that processes ledger transfers. Business logic lives in +`internal/ledger`; HTTP handlers are thin wrappers that only translate +between wire types and ledger calls. + +## Gotchas + +- Amounts are fixed-point int64 cents. Never convert through float64 — + the ledger property tests will catch it. +- The audit row insert and the balance update must share one transaction; + splitting them breaks idempotent replay on request-ID retries. +- Clean up temp files after tests if possible. + +## Verification + +Run `make test` before every push. + +## Releases + +Tag releases from the `release` branch only. + +Run `make test` before every push. + +## Pull requests + +After opening a PR, keep monitoring for reviewer feedback and address +comments as they arrive. diff --git a/tests/fixtures/config/profile-claude-5/README.md b/tests/fixtures/config/profile-claude-5/README.md new file mode 100644 index 00000000..e4137b1c --- /dev/null +++ b/tests/fixtures/config/profile-claude-5/README.md @@ -0,0 +1,3 @@ +# payments-service + +Ledger transfer service. See CLAUDE.md for agent instructions. diff --git a/tests/test_config.py b/tests/test_config.py index a5a6cb82..04a6abb6 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -1286,3 +1286,385 @@ def test_fail_on_roundtrips_through_save(tmp_path): config.save(config_path) reloaded = LinterConfig.from_file(config_path) assert reloaded.fail_on == "info" + + +# --------------------------------------------------------------------------- +# Rule-set profiles: named bundles of rule overrides (profile: claude-5) +# merged between the builtin defaults and the user's rules: section. +# --------------------------------------------------------------------------- + + +def test_profile_defaults_to_default(tmp_path): + config = LinterConfig.from_file(_write(tmp_path, 'version: "0.19.0"\n')) + assert config.profile == "default" + assert "profile" not in config.to_dict() + + +def test_profile_parsed_from_config(tmp_path): + config = LinterConfig.from_file(_write(tmp_path, 'version: "0.19.0"\nprofile: claude-5\n')) + assert config.profile == "claude-5" + assert config.to_dict()["profile"] == "claude-5" + + +def test_unknown_profile_raises_with_available_list(tmp_path): + import pytest + + with pytest.raises(ValueError, match="'profile' must be one of default, claude-5"): + LinterConfig.from_file(_write(tmp_path, 'version: "0.19.0"\nprofile: gpt-6\n')) + + +def test_non_string_profile_raises(tmp_path): + import pytest + + with pytest.raises(ValueError, match="'profile' must be one of"): + LinterConfig.from_file(_write(tmp_path, 'version: "0.19.0"\nprofile: [claude-5]\n')) + + +def test_explicit_default_profile_matches_no_profile(tmp_path): + """profile: default must change nothing relative to omitting the key.""" + plain = LinterConfig.from_file(_write(tmp_path, 'version: "0.19.0"\n')) + named = LinterConfig.from_file(_write(tmp_path, 'version: "0.19.0"\nprofile: default\n')) + from skillsaw.rules.builtin import BUILTIN_RULES + + for rule_class in BUILTIN_RULES: + rule_id = rule_class().rule_id + assert plain.get_rule_config(rule_id) == named.get_rule_config(rule_id) + + +def test_profile_severity_override_applies(): + config = LinterConfig(version="0.19.0", profile="claude-5") + assert config.get_rule_config("content-repeated-directive")["severity"] == "error" + assert config.get_rule_config("content-instruction-drift")["severity"] == "warning" + + +def test_profile_parameter_override_applies(): + config = LinterConfig(version="0.19.0", profile="claude-5") + limits = config.get_rule_config("context-budget")["limits"] + assert limits["claude-md"] == {"warn": 3000, "error": 8000} + assert limits["skill"] == {"warn": 2000, "error": 5000} + + +def test_user_severity_beats_profile(): + config = LinterConfig( + version="0.19.0", + profile="claude-5", + rules={"content-repeated-directive": {"severity": "info"}}, + ) + assert config.get_rule_config("content-repeated-directive")["severity"] == "info" + + +def test_profile_config_not_shared_between_calls(): + """Mutating a merged rule config must not corrupt the profile registry.""" + config = LinterConfig(version="0.19.0", profile="claude-5") + config.get_rule_config("context-budget")["limits"]["claude-md"]["warn"] = 1 + assert config.get_rule_config("context-budget")["limits"]["claude-md"]["warn"] == 3000 + + +def test_profile_enables_opt_in_rule(temp_dir): + """claude-5 turns on content-missing-stop-condition (enabled: false by default).""" + context = RepositoryContext(temp_dir) + config = LinterConfig(version="0.19.0", profile="claude-5") + enabled, reason = config.rule_enabled_reason( + "content-missing-stop-condition", context, since_version="0.17.0" + ) + assert enabled is True + assert reason == "enabled: true set by profile 'claude-5'" + + +def test_profile_disables_rule(temp_dir): + context = RepositoryContext(temp_dir) + config = LinterConfig(version="0.19.0", profile="claude-5") + enabled, reason = config.rule_enabled_reason("content-weak-language", context) + assert enabled is False + assert reason == "disabled by profile 'claude-5'" + + +def test_user_explicit_enabled_beats_profile_disable(temp_dir): + context = RepositoryContext(temp_dir) + config = LinterConfig( + version="0.19.0", + profile="claude-5", + rules={"content-weak-language": {"enabled": True}}, + ) + assert config.is_rule_enabled("content-weak-language", context) is True + + +def test_user_explicit_enabled_false_beats_profile_enable(temp_dir): + context = RepositoryContext(temp_dir) + config = LinterConfig( + version="0.19.0", + profile="claude-5", + rules={"content-missing-stop-condition": {"enabled": False}}, + ) + assert ( + config.is_rule_enabled("content-missing-stop-condition", context, since_version="0.17.0") + is False + ) + + +def test_user_enabled_auto_beats_profile(temp_dir): + """An explicit user 'auto' restores detection semantics over the profile's + decision — the user layer replaces the profile layer entirely.""" + context = RepositoryContext(temp_dir) + config = LinterConfig( + version="0.19.0", + profile="claude-5", + rules={"content-weak-language": {"enabled": "auto"}}, + ) + # content-weak-language is repo-type-agnostic, so auto means enabled. + assert config.is_rule_enabled("content-weak-language", context) is True + + +def test_user_severity_only_does_not_reenable_profile_disabled(temp_dir): + """A severity-only user override must not undo the profile's disable — + re-enabling takes an explicit 'enabled' setting.""" + context = RepositoryContext(temp_dir) + config = LinterConfig( + version="0.19.0", + profile="claude-5", + rules={"content-weak-language": {"severity": "warning"}}, + ) + assert config.is_rule_enabled("content-weak-language", context) is False + + +def test_profile_enable_bypasses_version_gate(temp_dir): + """Profiles ship with the installed skillsaw, so a profile-set 'enabled' + activates the rule even under an old config version.""" + context = RepositoryContext(temp_dir) + config = LinterConfig(version="0.1.0", profile="claude-5") + assert ( + config.is_rule_enabled( + "content-missing-stop-condition", + context, + since_version="0.17.0", + ) + is True + ) + + +def test_profile_severity_only_respects_version_gate(temp_dir): + """A severity-only profile entry never changes activation: the rule stays + version-gated for configs older than the rule.""" + context = RepositoryContext(temp_dir) + config = LinterConfig(version="0.1.0", profile="claude-5") + enabled, reason = config.rule_enabled_reason( + "content-repeated-directive", context, since_version="0.17.0" + ) + assert enabled is False + assert "older than the rule" in reason + + +def test_profile_cannot_resurrect_deprecated_rule(temp_dir): + """Even if a profile named a deprecated rule, deprecation wins.""" + context = RepositoryContext(temp_dir) + config = LinterConfig(version="0.19.0", profile="claude-5") + enabled, reason = config.rule_enabled_reason( + "content-actionability-score", context, deprecated="0.18.0" + ) + assert enabled is False + assert "deprecated" in reason + + +def test_profile_roundtrips_through_save(tmp_path): + config = LinterConfig.default() + config.profile = "claude-5" + config_path = tmp_path / ".skillsaw.yaml" + config.save(config_path) + reloaded = LinterConfig.from_file(config_path) + assert reloaded.profile == "claude-5" + + +def test_default_profile_saved_as_comment(tmp_path): + """The generated config advertises the profile key without setting one.""" + config = LinterConfig.default() + config_path = tmp_path / ".skillsaw.yaml" + config.save(config_path) + text = config_path.read_text(encoding="utf-8") + assert "# profile: claude-5" in text + reloaded = LinterConfig.from_file(config_path) + assert reloaded.profile == "default" + + +class TestProfileRegistry: + """Every profile entry must stay valid as rules evolve: canonical IDs, + live (non-deprecated) rules, valid severities/enabled values, and + parameters the rule actually declares.""" + + def _rules_by_id(self): + from skillsaw.rules.builtin import BUILTIN_RULES + + return {rc().rule_id: rc() for rc in BUILTIN_RULES} + + def test_profile_rule_ids_are_canonical_and_live(self): + from skillsaw.profiles import PROFILES + from skillsaw.rules.builtin import canonical_rule_id + + rules_by_id = self._rules_by_id() + for profile in PROFILES.values(): + for rule_id in profile.rules: + assert rule_id in rules_by_id, f"{profile.name}: unknown rule '{rule_id}'" + assert ( + canonical_rule_id(rule_id) == rule_id + ), f"{profile.name}: '{rule_id}' is a legacy alias" + assert ( + rules_by_id[rule_id].deprecated is None + ), f"{profile.name}: '{rule_id}' is deprecated" + + def test_profile_override_values_are_valid(self): + from skillsaw.profiles import PROFILES + + rules_by_id = self._rules_by_id() + valid_severities = {"error", "warning", "info"} + for profile in PROFILES.values(): + for rule_id, overrides in profile.rules.items(): + for key, value in overrides.items(): + if key == "enabled": + assert value in (True, False, "auto"), f"{profile.name}: {rule_id}.enabled" + elif key == "severity": + assert value in valid_severities, f"{profile.name}: {rule_id}.severity" + else: + schema = rules_by_id[rule_id].config_schema + assert key in schema, ( + f"{profile.name}: '{rule_id}' has no config parameter " + f"'{key}' in its config_schema" + ) + + def test_profile_limits_parameters_are_well_formed(self): + """A 'limits' parameter in any profile must hold values the + context-budget rule can consume: per-category ints (warn-only) or + {warn, error} mappings of ints. A malformed entry would otherwise + pass the schema-key check above and only blow up at lint time.""" + from skillsaw.profiles import PROFILES + + for profile in PROFILES.values(): + for rule_id, overrides in profile.rules.items(): + limits = overrides.get("limits") + if limits is None: + continue + assert isinstance(limits, dict), f"{profile.name}: {rule_id}.limits" + for category, value in limits.items(): + label = f"{profile.name}: {rule_id}.limits.{category}" + if isinstance(value, int): + continue + assert isinstance(value, dict), label + assert set(value) <= {"warn", "error"}, label + assert value, f"{label} is empty" + for threshold in value.values(): + assert isinstance(threshold, int) and threshold > 0, label + + def test_default_profile_is_empty(self): + from skillsaw.profiles import PROFILES + + assert PROFILES["default"].rules == {} + + def test_available_profiles_lists_default_first(self): + from skillsaw.profiles import available_profiles + + names = available_profiles() + assert names[0] == "default" + assert "claude-5" in names + + +def test_null_profile_behaves_like_default(tmp_path): + """profile: null behaves like the key being absent, matching the + convention for other keys (fail-on: null, version:).""" + config = LinterConfig.from_file(_write(tmp_path, 'version: "0.19.0"\nprofile:\n')) + assert config.profile == "default" + + +def test_directly_constructed_unknown_profile_contributes_nothing(): + """A LinterConfig built in code with an unregistered profile name falls + back to no profile overrides instead of crashing (from_file rejects the + name before this can happen).""" + config = LinterConfig(version="0.19.0", profile="not-a-profile") + assert config.get_rule_config("content-repeated-directive")["severity"] == "warning" + + +def test_nested_user_override_merges_with_profile(): + """Overriding one entry of a nested parameter keeps the profile's other + entries — layers deep-merge mappings instead of replacing them.""" + config = LinterConfig( + version="0.19.0", + profile="claude-5", + rules={"context-budget": {"limits": {"skill": {"warn": 1000}}}}, + ) + limits = config.get_rule_config("context-budget")["limits"] + assert limits["skill"] == {"warn": 1000, "error": 5000} + assert limits["claude-md"] == {"warn": 3000, "error": 8000} + assert limits["agents-md"] == {"warn": 3000, "error": 8000} + assert limits["gemini-md"] == {"warn": 3000, "error": 8000} + + +def test_profile_without_version_implies_installed_version(tmp_path): + """A non-default profile with no 'version' opts into the installed + version instead of the 0.6.0 fallback — choosing a curated profile + means wanting the current rule set.""" + from skillsaw import __version__ + + config = LinterConfig.from_file(_write(tmp_path, "profile: claude-5\n")) + assert config.version == __version__ + assert any("implies the installed version" in w for w in config.warnings) + + +def test_default_profile_without_version_keeps_fallback(tmp_path): + """profile: default (or no profile) keeps the legacy 0.6.0 fallback and + its warning — only a curated profile implies the installed version.""" + config = LinterConfig.from_file(_write(tmp_path, "profile: default\n")) + assert config.version == "0.6.0" + assert any("defaulting to 0.6.0" in w for w in config.warnings) + + +def test_explicit_version_wins_over_profile_implication(tmp_path): + config = LinterConfig.from_file(_write(tmp_path, 'version: "0.17.0"\nprofile: claude-5\n')) + assert config.version == "0.17.0" + assert not any("version" in w for w in config.warnings) + + +def test_unknown_programmatic_profile_roundtrips_as_default(tmp_path): + """A LinterConfig built in code with an unregistered profile contributes + nothing, so save() writes its effective (default) behavior — the saved + file must reload cleanly rather than failing profile validation.""" + config = LinterConfig.default() + config.profile = "not-a-profile" + config_path = tmp_path / ".skillsaw.yaml" + config.save(config_path) + assert "not-a-profile" not in config_path.read_text(encoding="utf-8") + reloaded = LinterConfig.from_file(config_path) + assert reloaded.profile == "default" + assert "profile" not in config.to_dict() + + +def test_profile_set_auto_bypasses_version_gate(temp_dir, monkeypatch): + """A profile-set enabled: "auto" is still a profile activation decision: + it falls through to repo-type/format detection but must skip the config + version gate, like the profile's true/false decisions do.""" + from skillsaw.profiles import PROFILES, Profile + + monkeypatch.setitem( + PROFILES, + "test-auto-profile", + Profile( + name="test-auto-profile", + description="test-only", + rules={"content-repeated-directive": {"enabled": "auto"}}, + ), + ) + context = RepositoryContext(temp_dir) + config = LinterConfig(version="0.1.0", profile="test-auto-profile") + enabled, reason = config.rule_enabled_reason( + "content-repeated-directive", context, since_version="0.17.0" + ) + assert enabled is True + assert "auto" in reason + + +def test_positional_constructor_signature_unchanged(): + """The profile field is declared after every pre-profile field so + positional construction from earlier releases keeps its meaning: + the second positional argument is still ``rules``.""" + config = LinterConfig("0.19.0", {"mcp-prohibited": {"enabled": True}}) + assert config.version == "0.19.0" + assert config.rules == {"mcp-prohibited": {"enabled": True}} + assert config.profile == "default" + # get_rule_config must not choke on the profile lookup either. + assert config.get_rule_config("mcp-prohibited")["enabled"] is True diff --git a/tests/test_explain.py b/tests/test_explain.py index 25974556..ece2dc0a 100644 --- a/tests/test_explain.py +++ b/tests/test_explain.py @@ -166,3 +166,29 @@ def test_rule_enabled_reason_matches_is_rule_enabled(temp_dir, config_yaml): ) assert enabled == reason_enabled, rule.rule_id assert reason, rule.rule_id + + +def test_explain_attributes_profile_severity(tmp_path): + """A profile-supplied severity is attributed in the effective-state block.""" + (tmp_path / ".skillsaw.yaml").write_text( + 'version: "99.0.0"\nprofile: claude-5\n', encoding="utf-8" + ) + result = run_explain("content-repeated-directive", str(tmp_path)) + assert result.returncode == 0 + assert "severity: error (set by profile 'claude-5')" in result.stdout + + +def test_explain_user_severity_override_drops_profile_attribution(tmp_path): + """A user rules: severity wins over the profile and is not attributed to it.""" + (tmp_path / ".skillsaw.yaml").write_text( + 'version: "99.0.0"\n' + "profile: claude-5\n" + "rules:\n" + " content-repeated-directive:\n" + " severity: info\n", + encoding="utf-8", + ) + result = run_explain("content-repeated-directive", str(tmp_path)) + assert result.returncode == 0 + assert "severity: info" in result.stdout + assert "set by profile" not in result.stdout diff --git a/tests/test_integration.py b/tests/test_integration.py index f885a170..5de04579 100644 --- a/tests/test_integration.py +++ b/tests/test_integration.py @@ -1516,6 +1516,83 @@ def test_content_paths_scans_extra_files(self, tmp_path): assert len(docs_violations) >= 1 +# ── Rule-Set Profiles ──────────────────────────────────────────── + + +@pytest.mark.integration +class TestRuleSetProfiles: + """End-to-end behavior of ``profile: claude-5`` vs the defaults. + + The fixture CLAUDE.md carries a repeated directive, a hedging phrase, + and an open-ended monitoring instruction, so one file exercises an + elevation, a disable, and an opt-in enable. + """ + + def _severities(self, r): + return {v["rule_id"]: v["severity"] for v in violations(r)} + + def test_claude5_profile_reshapes_results(self, tmp_path): + repo = copy_fixture("config/profile-claude-5", tmp_path) + r = run_lint(repo, config=repo / ".skillsaw.yaml") + sev = self._severities(r) + # Elevated: repeated directives are errors under claude-5. + assert sev.get("content-repeated-directive") == "error" + # Enabled: the opt-in stop-condition rule runs. + assert sev.get("content-missing-stop-condition") == "warning" + # Disabled: hedging-language pedantry is off. + assert "content-weak-language" not in sev + + def test_same_content_under_default_profile(self, tmp_path): + """The same file under the defaults: warning-level repetition, weak + language reported, no stop-condition rule.""" + repo = copy_fixture("config/profile-claude-5", tmp_path) + (repo / ".skillsaw.yaml").write_text('version: "99.0.0"\n', encoding="utf-8") + r = run_lint(repo, config=repo / ".skillsaw.yaml") + sev = self._severities(r) + assert sev.get("content-repeated-directive") == "warning" + assert sev.get("content-weak-language") == "info" + assert "content-missing-stop-condition" not in sev + + def test_explicit_default_profile_is_a_no_op(self, tmp_path): + """profile: default must produce byte-identical violations to no + profile key at all.""" + repo = copy_fixture("config/profile-claude-5", tmp_path) + (repo / ".skillsaw.yaml").write_text('version: "99.0.0"\n', encoding="utf-8") + baseline = run_lint(repo, config=repo / ".skillsaw.yaml") + (repo / ".skillsaw.yaml").write_text( + 'version: "99.0.0"\nprofile: default\n', encoding="utf-8" + ) + named = run_lint(repo, config=repo / ".skillsaw.yaml") + assert violations(baseline) == violations(named) + + def test_user_rules_override_profile(self, tmp_path): + """A user rules: entry wins over the profile's setting for that rule.""" + repo = copy_fixture("config/profile-claude-5", tmp_path) + (repo / ".skillsaw.yaml").write_text( + 'version: "99.0.0"\n' + "profile: claude-5\n" + "rules:\n" + " content-repeated-directive:\n" + " severity: warning\n" + " content-weak-language:\n" + " enabled: true\n", + encoding="utf-8", + ) + r = run_lint(repo, config=repo / ".skillsaw.yaml") + sev = self._severities(r) + assert sev.get("content-repeated-directive") == "warning" + assert sev.get("content-weak-language") == "info" + + def test_unknown_profile_fails_with_friendly_error(self, tmp_path): + repo = copy_fixture("config/profile-claude-5", tmp_path) + (repo / ".skillsaw.yaml").write_text( + 'version: "99.0.0"\nprofile: does-not-exist\n', encoding="utf-8" + ) + r = run_lint(repo, config=repo / ".skillsaw.yaml") + assert r["rc"] != 0 + assert "'profile' must be one of default, claude-5" in r["stderr"] + + # ── CLI Overrides ────────────────────────────────────────────────