release: development cycle — PR #50 bug-fix batch + agent skills + housekeeping

.agents/skills/add-config-field/SKILL.md

@@ -0,0 +1,155 @@
+---
+name: add-config-field
+description: Use this skill when adding a new YAML configuration field to ForgeLM. Handles Pydantic model update, cross-field validation, config_template.yaml sync, bilingual docs, and tests. Triggered by requests like "add a new config option for X", "expose Y as a YAML field", "make Z configurable".
+---
+
+# Skill: Add a New Config Field
+
+ForgeLM is strictly config-driven. Every new runtime behaviour becomes a YAML field passing through Pydantic validation to the consumer. This skill walks through all the places you must touch so nothing drifts.
+
+## When to use
+
+- User wants a new YAML option (e.g., `training.new_flag: true`)
+- A feature is currently hardcoded and needs to be exposed
+- A new optional dependency needs its own config section
+
+Do **not** use for:
+- Changing an existing field's default (that's a different, more risky change)
+- Internal refactors that don't affect user-visible YAML
+
+## Required reading before acting
+
+1. [docs/standards/architecture.md](../../../docs/standards/architecture.md) — config flow principle
+2. [docs/standards/coding.md](../../../docs/standards/coding.md) — Pydantic conventions
+3. [forgelm/config.py](../../../forgelm/config.py) — existing patterns to match
+
+## Steps
+
+### 1. Pick the right config class
+
+Look at [forgelm/config.py](../../../forgelm/config.py). Choose:
+
+- `ModelConfig` — for model-related fields (name, quantization, MoE)
+- `LoraConfigModel` — for LoRA/DoRA/PiSSA/rsLoRA parameters
+- `TrainingConfig` — for training hyperparameters, trainer types, algorithmic flags
+- `DataConfig` — for dataset paths, format, preprocessing
+- `EvaluationConfig` / `SafetyConfig` / `JudgeConfig` — for eval pipeline
+- `ComplianceConfig` — for EU AI Act artifacts
+- `WebhookConfig`, `TrackingConfig`, etc. — for integrations
+- `ForgeConfig` (root) — only if genuinely cross-cutting
+
+If none fit, the field may belong to a **new** config class — see architecture.md §1.
+
+### 2. Add the field
+
+```python
+# forgelm/config.py
+class TrainingConfig(BaseModel):
+    ...
+    # existing fields
+
+    new_flag: Optional[bool] = None
+    """If set, enable the X behaviour. Default: inherit from model's default."""
+```
+
+Rules:
+- Use `Optional[T] = None` for truly optional fields; `T = default_value` for always-set fields with safe defaults.
+- Use `Literal["a", "b"]` for enums, not `str`.
+- Field order: existing fields first, new field at a logical group boundary.
+- One-line docstring directly below the field (Pydantic doesn't use these, but humans and docs do).
+
+### 3. Validation
+
+If the field has invariants, add `field_validator` or `model_validator`:
+
+```python
+@field_validator("new_flag")
+@classmethod
+def _validate_new_flag(cls, v: Optional[bool], info) -> Optional[bool]:
+    if v and info.data.get("trainer_type") != "grpo":
+        raise ValueError("new_flag is only valid for trainer_type='grpo'")
+    return v
+```
+
+Error messages follow [error-handling.md](../../../docs/standards/error-handling.md) — specific, actionable.
+
+### 4. Wire it to the consumer
+
+Find the module that will read the field:
+
+- Training-loop flags → [forgelm/trainer.py](../../../forgelm/trainer.py)
+- Model-loading flags → [forgelm/model.py](../../../forgelm/model.py)
+- Data flags → [forgelm/data.py](../../../forgelm/data.py)
+- Safety flags → [forgelm/safety.py](../../../forgelm/safety.py)
+
+Access via `config.training.new_flag` — never import from environment or global state.
+
+### 5. Update `config_template.yaml`
+
+The repo ships [config_template.yaml](../../../config_template.yaml) as the canonical example. Add your field with a comment:
+
+```yaml
+training:
+  trainer_type: sft
+  ...
+  # If set, enables X. See docs/reference/configuration.md#new_flag
+  new_flag: false
+```
+
+### 6. Document the field
+
+Both mirrors:
+
+- [docs/reference/configuration.md](../../../docs/reference/configuration.md) — add to the appropriate section with an example
+- [docs/reference/configuration-tr.md](../../../docs/reference/configuration-tr.md) — matching Turkish section
+
+Follow [localization.md](../../../docs/standards/localization.md) for the TR mirror.
+
+### 7. Write a test
+
+Create or extend [tests/test_config.py](../../../tests/test_config.py):
+
+```python
+def test_new_flag_defaults_to_none():
+    cfg = ForgeConfig.model_validate(minimal_config())
+    assert cfg.training.new_flag is None
+
+def test_new_flag_requires_grpo_trainer():
+    with pytest.raises(ValidationError, match="trainer_type='grpo'"):
+        ForgeConfig.model_validate(
+            minimal_config(training={"new_flag": True, "trainer_type": "sft"})
+        )
+```
+
+At least one happy-path + one error-path test.
+
+### 8. Update CHANGELOG
+
+In [CHANGELOG.md](../../../CHANGELOG.md), under `[Unreleased]` / `### Added`:
+
+```
+- **New config field**: `training.new_flag` — enables X for GRPO trainer. See docs/reference/configuration.md#new_flag.
+```
+
+## Verification before PR
+
+```bash
+pytest tests/test_config.py -v
+ruff check forgelm/config.py
+forgelm --config config_template.yaml --dry-run
+```
+
+All three must pass.
+
+## Pitfalls to avoid
+
+- **Adding `new_flag` without validation.** Pydantic accepts any value of the declared type — if there are interactions with other fields, *you* must validate them.
+- **Skipping the TR mirror.** The PR gets rejected. Same change, same PR.
+- **Breaking backward compatibility with a default change.** If users have `training:` blocks that worked before and would fail now, you need a major bump. Consider adding as opt-in instead.
+- **Forgetting `config_template.yaml`.** The CI dry-run uses this; if your field is required, the template must include it.
+- **Logging the field value.** If it could contain secrets (tokens, paths), sanitize per [logging-observability.md](../../../docs/standards/logging-observability.md).
+
+## Related skills
+
+- `add-trainer-feature` — if the field controls end-to-end behaviour, not just a knob
+- `sync-bilingual-docs` — run after step 6 to verify TR/EN parity

.agents/skills/add-test/SKILL.md

@@ -0,0 +1,206 @@
+---
+name: add-test
+description: Use this skill when writing new tests for ForgeLM. Applies conventions from docs/standards/testing.md — fixture factory patterns, mock boundaries, no-GPU-no-network discipline, coverage floor. Triggered by requests like "add a test for X", "cover function Y", "write a test that triggers error Z".
+---
+
+# Skill: Write a Test for ForgeLM
+
+Tests in ForgeLM are pragmatic: fast, deterministic, laptop-runnable. This skill encodes the conventions so new tests fit in and CI stays green.
+
+## When to use
+
+- Covering a new function, class, or CLI flag
+- Adding regression tests for a fixed bug
+- Extending test_config.py with a new validation check
+
+Do **not** use for:
+- Actually running tests (just run `pytest tests/`)
+- Performance benchmarks (separate concern — not in `tests/`)
+
+## Required reading before writing
+
+1. [docs/standards/testing.md](../../../docs/standards/testing.md) — full rules
+2. [tests/conftest.py](../../../tests/conftest.py) — the `minimal_config` factory
+3. **A similar existing test file** — e.g., if you're writing `test_new_feature.py`, read `test_alignment.py` first
+
+## Decide: which test file?
+
+| Type of code being tested | Test file |
+|---|---|
+| Pydantic config field or validator | `test_config.py` |
+| A new alignment trainer (DPO-style) | `test_alignment.py` |
+| A full pipeline scenario | `test_integration.py` |
+| CLI argument parsing, exit codes | `test_cli.py` or `test_cli_subcommands.py` |
+| Compliance artifact generation | `test_compliance.py` or `test_eu_ai_act.py` |
+| Safety evaluation | `test_safety_advanced.py` |
+| Webhook behaviour | `test_webhook.py` |
+| Something that doesn't fit | New `test_<module>.py` matching the source module |
+
+Prefer extending an existing file over creating a new one — keeps the test layout predictable.
+
+## Template
+
+```python
+"""Tests for forgelm.<module> — <short scope description>."""
+import pytest
+from forgelm.<module> import <public_api>
+from forgelm.config import ForgeConfig
+from tests.conftest import minimal_config
+
+
+class TestPublicFunction:
+    """Group related tests in a class for readability."""
+
+    def test_happy_path(self, tmp_path):
+        result = <public_api>(...)
+        assert result.status == "ok"
+        assert result.field == expected_value
+
+    def test_raises_on_invalid_input(self):
+        with pytest.raises(ValueError, match="specific error keyword"):
+            <public_api>(bad_input)
+
+    @pytest.mark.parametrize("value,expected", [
+        ("a", "result_a"),
+        ("b", "result_b"),
+    ])
+    def test_parametrized(self, value, expected):
+        assert <public_api>(value) == expected
+```
+
+## Fixtures — use, don't reinvent
+
+```python
+# Minimal valid config:
+cfg_dict = minimal_config()
+
+# With overrides:
+cfg_dict = minimal_config(training={"trainer_type": "dpo", "dpo_beta": 0.2})
+
+# Parsed into Pydantic:
+cfg = ForgeConfig.model_validate(cfg_dict)
+```
+
+Don't build config dicts from scratch. Don't add a new top-level fixture to `conftest.py` unless it's reused across ≥3 test files.
+
+## Mocking rules
+
+From [testing.md](../../../docs/standards/testing.md):
+
+**Always mock:**
+- Network: `requests`, `huggingface_hub` downloads, OpenAI/Anthropic APIs
+- GPU: `torch.cuda.is_available`, `torch.cuda.get_device_name`
+- Expensive imports: `unsloth`, `bitsandbytes`, `deepspeed`, `lm_eval` when they cause import errors
+
+**Never mock:**
+- Pydantic validation (use real config objects)
+- File I/O under `tmp_path`
+- YAML parsing
+- Internal `forgelm.*` modules
+
+### Mocking patterns
+
+```python
+# Using pytest monkeypatch:
+def test_no_gpu_path(monkeypatch):
+    monkeypatch.setattr("torch.cuda.is_available", lambda: False)
+    result = model.load(cfg)
+    assert result.device == "cpu"
+
+# Using mock.patch:
+from unittest.mock import patch, MagicMock
+
+@patch("forgelm.safety.pipeline")
+def test_safety_eval_returns_score(mock_pipeline):
+    mock_pipeline.return_value = MagicMock(return_value=[{"label": "safe", "score": 0.98}])
+    result = safety.evaluate(...)
+    assert result.safety_score > 0.9
+
+# Patching requests:
+from unittest.mock import patch
+
+def test_webhook_failure_doesnt_raise(caplog):
+    with patch("forgelm.webhook.requests.post") as mock_post:
+        mock_post.side_effect = requests.RequestException("boom")
+        notifier.send(...)  # must not raise
+    assert "webhook delivery failed" in caplog.text.lower()
+```
+
+## Assertions
+
+Prefer specific over general:
+
+| ❌ Weak | ✅ Specific |
+|---|---|
+| `assert result` | `assert result.status == "success"` |
+| `assert len(items) > 0` | `assert len(items) == 3` |
+| `assert "error" in output` | `assert "trainer_type must be one of" in output` |
+
+`pytest.raises(match="...")` is better than just `pytest.raises` — catches regressions where the error type stays but the message drifts.
+
+## Testing error paths
+
+From [error-handling.md](../../../docs/standards/error-handling.md):
+
+```python
+def test_invalid_trainer_type_exit_code(tmp_path, monkeypatch):
+    config_path = tmp_path / "bad.yaml"
+    config_path.write_text("training:\n  trainer_type: 'nonexistent'\n...")
+    # Use subprocess to get real exit code:
+    result = subprocess.run(
+        ["forgelm", "--config", str(config_path), "--dry-run"],
+        capture_output=True, text=True
+    )
+    assert result.returncode == 1  # EXIT_CONFIG_ERROR
+    assert "trainer_type" in result.stderr
+```
+
+Or in-process with `capsys`:
+
+```python
+def test_cli_config_error_exits_1(capsys):
+    with pytest.raises(SystemExit) as exc_info:
+        cli.main(["--config", "bad.yaml", "--dry-run"])
+    assert exc_info.value.code == 1
+    captured = capsys.readouterr()
+    assert "config" in captured.err.lower()
+```
+
+## Coverage
+
+- Every new public function needs ≥1 test covering the success path.
+- Every `raise` statement and every `sys.exit(!=0)` needs ≥1 test triggering it.
+- `pragma: no cover` only for:
+  - `if __name__ == "__main__":` blocks
+  - `except ImportError:` fallbacks for optional deps
+  - Documented platform-specific branches
+
+Current coverage floor is **40%** (enforced via [`pyproject.toml`](../../../pyproject.toml) `[tool.pytest.ini_options].addopts → --cov-fail-under=40`). Treat it as a floor not a target — you're usually well above. If you ever quote the number elsewhere in docs or skills, source it from `pyproject.toml` to prevent drift.
+
+## Before opening the PR
+
+```bash
+# Your new tests pass:
+pytest tests/test_<your_file>.py -v
+
+# No coverage regression:
+pytest --cov=forgelm --cov-report=term-missing tests/
+
+# Linter happy:
+ruff check tests/test_<your_file>.py
+ruff format --check tests/test_<your_file>.py
+```
+
+## Pitfalls
+
+- **`pytest.skip("TODO")`** — tracks nothing. Use `pytest.mark.xfail(reason="...", strict=False)` with a linked issue.
+- **Sleep-based waits** — `time.sleep(2)` makes tests flaky. Mock time instead.
+- **Module-level state in tests** — `shared_state = []` at test-file scope causes cross-test pollution.
+- **Testing the mock** — if your assertion is "mock was called with X", make sure the real-world contract actually requires X. Otherwise you're testing the mock, not the code.
+- **Session-scoped fixtures with mutable state** — pollution across test runs. Scope to `function` unless the fixture is immutable.
+
+## Related skills
+
+- `add-config-field` — when the thing you're testing is a config field
+- `add-trainer-feature` — when the thing you're testing is a larger feature
+- `review-pr` — run its checklist before requesting review