diff --git a/.claude/skills/add-trainer-feature/SKILL.md b/.claude/skills/add-trainer-feature/SKILL.md index 0d101b11..2cf08bc2 100644 --- a/.claude/skills/add-trainer-feature/SKILL.md +++ b/.claude/skills/add-trainer-feature/SKILL.md @@ -24,7 +24,7 @@ Do **not** use for: 1. [docs/standards/architecture.md](../../../docs/standards/architecture.md) — module boundaries 2. [docs/standards/testing.md](../../../docs/standards/testing.md) — what tests CI demands -3. The **phase file** describing the feature (e.g., [docs/roadmap/phase-10-post-training.md](../../../docs/roadmap/phase-10-post-training.md)) +3. The **phase file** describing the feature (e.g., [docs/roadmap/completed-phases.md](../../../docs/roadmap/completed-phases.md)) 4. Existing similar feature — read one similar trainer/module end-to-end before writing yours ## The scaffold diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 6e33adfc..bf070e0a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -228,6 +228,17 @@ jobs: # for local feedback during a docs PR. run: python3 tools/check_cli_help_consistency.py --strict + - name: Unguarded sys.modules.pop guard + # v0.5.7 round-4 absorption: the v0.5.7 round-3 review traced + # 35 spurious full-suite failures to three test sites that + # popped torch / numpy from sys.modules without restoring + # them, half-loading torch._C for every later test in the + # pytest session. This guard fails CI on any new + # ``sys.modules.pop("")`` / + # ``del sys.modules[""]`` site, steering authors + # to ``monkeypatch.delitem`` (auto-restores on teardown). + run: python3 tools/check_no_unguarded_sys_modules_pop.py + - name: License check run: | test -f LICENSE || (echo "LICENSE file missing" && exit 1) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5f188866..6a4d5500 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,148 @@ All notable changes to ForgeLM are documented here. ## [Unreleased] +Review-absorption rounds 3 + 4 follow-up to v0.5.7. No public surface +change, but training-pipeline UX, doctor robustness, and test-isolation +guarantees improve. + +### Added + +- **Early-fail ABI preflight in the training pipeline.** A new shared + helper at `forgelm/cli/_abi_check.py` runs the same numpy/torch + pairing check as `forgelm doctor`'s `numpy.torch_abi` probe, and + `forgelm/cli/_training.py::_run_training_pipeline` now calls it + before importing the heavy stack. An operator who hits a residual + Intel Mac NumPy 2 mismatch (drifted env, out-of-band + `pip install -U numpy`) now sees a single-line error with the exact + remediation command (`pip install 'numpy<2'`) instead of a cryptic + `NameError: name '_C' is not defined` from torch mid-training. The + PEP 508 marker in `pyproject.toml` remains the primary fix for + fresh installs; the preflight is the second line of defense. The + preflight's JSON envelope is now documented in + [`docs/usermanuals/{en,tr}/reference/json-output.md`](docs/usermanuals/en/reference/json-output.md) + so CI consumers can branch on the stable `"error": + "numpy_torch_abi_mismatch"` token. +- **`tools/check_no_unguarded_sys_modules_pop.py`** — new CI guard + fails on `sys.modules.pop("torch"|"numpy"|"trl"|…)` or + `del sys.modules["…"]` without `monkeypatch.delitem` for tracked + teardown. Wired into the self-review gauntlet + ([`CLAUDE.md`](CLAUDE.md)) and `.github/workflows/ci.yml`. Round-3 + traced 35 spurious full-suite failures to this pattern; the guard + makes the regression impossible to silently re-introduce. +- **`pytest-randomly` added to `[dev]` extra.** Shuffles test order + per session so order-dependent bugs (the exact failure mode round-3 + fixed) surface in CI instead of waiting for a Python micro-bump or + runner change. Reproduce a specific shuffle with + `pytest --randomly-seed=`. + +### Fixed + +- **`forgelm/trainer.py::_get_training_args_for_type`** — SFT branch + now raises `ValueError` (not warn-and-continue) when neither + `max_length` nor `max_seq_length` is exposed on `SFTConfig`. Silent + drop of an explicit `model.max_length` YAML setting would violate + `docs/standards/error-handling.md` "no silent failures" — error + message lists the actually-detected parameters for forensics. The + trl version is probed off `SFTConfig.__module__` rather than a + duplicate `import trl` (round-4 cleanup). +- **`tests/test_doctor.py` + `tests/test_quickstart_compat.py`** — + three pre-existing tests that popped `sys.modules['torch']` / + `sys.modules['numpy']` without restoring them now use + `monkeypatch.delitem` for tracked teardown. The stranded modules + half-initialised every subsequent `import torch` in the pytest + session (`torch._C` never re-bound), causing every downstream + `from trl import SFTConfig` and a long tail of merging / moe / + quickstart / wizard / judge / safety tests to fail in the full + suite even though they passed in isolation. Pure test-only fix; no + production behaviour change. +- **`forgelm/cli/subcommands/_doctor.py::_check_numpy_torch_abi`** — + refactored to call into the shared `_abi_check.py` helper rather + than duplicating the version-parsing + threshold logic. Docstring + tightened to honestly note that torch's C++ `_ARRAY_API not found` + message is outside Python's warnings machinery and `fprintf(stderr, + …)`-based suppression would require fd-level redirection (which is + unsafe in a long-lived CLI process). Module-level `re` / `warnings` + imports promoted from function-local since the shared helper now + owns them. The exhaustive-enum guard at the end of the function now + raises `RuntimeError` instead of `assert` so `python -O` doesn't + strip the check. +- **`forgelm/cli/_abi_check.py::compute_numpy_torch_abi_status`** — + fail-safe short-circuit added for `(0, 0)` version-parse fallback + on either side. A corporate fork with a non-semver torch tag (e.g. + `foo-bar`) would otherwise have its `_major_minor` collapse to + `(0, 0)`, trip `< (2, 3)`, and trigger a false-positive + `ABI_BROKEN` against any healthy numpy ≥ 2. Now: if either parses + to `(0, 0)`, treat as `ABI_OK` ("version unknown, do not + classify"). `format_abi_remediation` also gained an explicit + precondition check so a future call-site drift can't silently + emit `"torch None …"` strings. + +### Internal + +- Test docstring in [`tests/test_abi_check.py`](tests/test_abi_check.py) + now quotes the actual PEP 508 marker from `pyproject.toml` + (`sys_platform == 'darwin'`) instead of the equivalent-but-different + `platform_system == "Darwin"` form, so future reviewers chasing the + marker chain don't conclude it has drifted. +- One leftover `(Faz 12)` in the v0.5.2 CHANGELOG entry rewritten to + `(Phase 12)` — English-only-in-code-and-CHANGELOG cleanup. + +## [0.5.7] — 2026-05-10 + +Patch release. Fixes a runtime `TypeError` in the SFT trainer that +prevented every SFT training run from starting on modern `trl` +versions (0.13 and the 1.x line). The `max_seq_length` parameter was +renamed to `max_length` on `SFTConfig` in trl 0.13 and the old name +removed; the v0.5.6 release was still passing `max_seq_length` +unconditionally, so `forgelm --config ` would crash with +`TypeError: SFTConfig.__init__() got an unexpected keyword argument +'max_seq_length'` on any environment that pulled a current trl wheel +(notably the Colab default `pip install forgelm` path). + +Also closes the NumPy 2.x ↔ torch 2.2 binary-ABI mismatch that the +v0.5.6 Intel Mac torch revert exposed. With `torch>=2.2.0` as the +floor and no upper-pin on numpy, pip resolved `numpy>=2` on Intel Mac +hosts; torch 2.2 was compiled against NumPy 1.x and silently +degraded its numpy bridge with `_ARRAY_API not found`. v0.5.7 adds a +platform-scoped marker pin (`numpy<2; sys_platform == 'darwin' and +platform_machine == 'x86_64'`) plus a new doctor probe that surfaces +the mismatch as a structured `fail` instead of a stderr-only +UserWarning. + +### Added + +- **`forgelm doctor` — `numpy.torch_abi` probe.** New diagnostic + inspects torch + numpy major-version pairing and emits a `fail` + with a `pip install 'numpy<2'` remediation hint when a torch < 2.3 + install is paired with NumPy ≥ 2 (the canonical Intel Mac install + failure mode). Probe imports torch / numpy with warnings suppressed + so the probe itself does not pollute stderr with the very + `_ARRAY_API not found` warning it is trying to detect. Surfaces in + the `forgelm doctor --output-format json` envelope so CI consumers + catch the issue programmatically. + +### Fixed + +- **`forgelm/trainer.py::_get_training_args_for_type`** — the SFT + branch now inspects `SFTConfig.__init__`'s signature at runtime and + picks the correct sequence-length-cap parameter name. trl 0.13+ + (including the 1.x line) receives `max_length`; trl 0.12.x — still + within the `pyproject.toml` floor `trl>=0.12.0,<2.0.0` — keeps + receiving `max_seq_length`. No code change for DPO / SimPO / KTO / + ORPO / GRPO trainers (their `*Config` parameters were not affected + by the rename). +- **`tests/test_trainer_sft_config.py`** (new) — three regression + tests pin the modern-trl path (`max_length`), the legacy-trl path + (`max_seq_length`), and that `packing` / `dataset_text_field` + continue to be propagated. +- **`pyproject.toml` — Intel Mac (x86_64) NumPy 2 ABI mismatch.** + Added `numpy<2; sys_platform == 'darwin' and platform_machine == + 'x86_64'` so pip's resolver caps numpy at the 1.x line on the only + platform that is wheel-locked to torch 2.2.x. Other platforms keep + numpy unconstrained — torch 2.3+ on Linux / Apple Silicon / Windows + is binary-compatible with NumPy 2.x. Affected `pip install forgelm` + users on Intel Mac since the v0.5.6 torch revert. + ## [0.5.6] — 2026-05-10 **Status:** Released to PyPI 2026-05-10 via the cross-OS publish @@ -412,7 +554,7 @@ collectors / BYOD / IO concerns. refusal, secret-mask error path, TLS verify). Used by `forgelm doctor` and any future probe / telemetry / registry ping that needs an outbound read. -- **`forgelm/_http.safe_post`** (Faz 7 carry-over) — single boundary +- **`forgelm/_http.safe_post`** — single boundary for outbound HTTP with SSRF guard, redirect refusal, scheme policy, timeout floor, TLS pinning, secret-mask error reasons. Migrated webhook + judge + synthetic call sites. @@ -782,7 +924,7 @@ review-cycle fixes applied across the four phases above). ### Phase 12.5 — Data Curation Polish (backlog items #1–#4) Four follow-up items from -[`docs/roadmap/phase-12-5-backlog.md`](docs/roadmap/phase-12-5-backlog.md) +[`docs/roadmap/completed-phases.md`](docs/roadmap/completed-phases.md) ship together — none require new architecture; each is a small additive surface on top of the Phase 12 ingestion + audit lineage. @@ -1115,7 +1257,7 @@ Second-round review of the Phase 12 commit surfaced 22 findings spanning correct - **`_row_quality_flags` typed `Optional[str]`** — The function already accepted `None` at runtime; the signature now reflects that and the test's `# type: ignore[arg-type]` suppression is gone. - **Cognitive-complexity refactors** — `_row_quality_flags` (CCN 22 → ≤ 10 via per-check helpers `_check_low_alpha_ratio` / `_check_low_punct_endings` / `_check_abnormal_mean_word_length` / `_check_short_paragraphs` / `_check_repeated_lines`); `find_near_duplicates_minhash` (CCN 21 → ≤ 10 via `_build_minhash_lsh` + `_emit_minhash_pair`); `audit_dataset` (CCN 21 → ≤ 12 via `_fold_outcome_into_summary` + `_build_quality_summary` + `_build_near_duplicate_summary`). - **Regex / lint code-smells** — `[A-Za-z0-9_]` → `\w` in the GitHub PAT pattern; `[ ]{0,3}` → ` {0,3}` (single-char class collapsed) in markdown patterns; `\s` → `[ \t]` in heading pattern (mitigates the polynomial-backtracking concern SonarCloud flagged); duplicate `"chunk_tokens must be positive"` / `"max_chunk_size must be positive"` literal strings extracted to module constants `_CHUNK_TOKENS_POSITIVE_MSG` / `_CHUNK_SIZE_POSITIVE_MSG`; `_MARKDOWN_OVERLAP_UNSUPPORTED_MSG` constant for the new validator; comprehension `["| " + " | ".join(c for c in row) + " |"]` simplified to `["| " + " | ".join(row) + " |"]`. -- **Documentation parity** — `docs/guides/data_audit.md` quality-filter bullet list and JSON example now include `repeated_lines` and a note about code-fence stripping. `docs/guides/ingestion-tr.md` mirrors the EN guide's chunking-strategies table (markdown row added) and gains a new "secrets/credential masking (Faz 12)" section. `CHANGELOG`'s Phase 12 entry no longer overstates the `[ingestion-secrets]` extra: the regex set is the sole detection backend in v0.5.2, and the `detect-secrets` package is reserved for a follow-up release. `README` separates "From PyPI" and "From a local clone" install blocks so copy-paste users don't hit `-e .` confusion. +- **Documentation parity** — `docs/guides/data_audit.md` quality-filter bullet list and JSON example now include `repeated_lines` and a note about code-fence stripping. `docs/guides/ingestion-tr.md` mirrors the EN guide's chunking-strategies table (markdown row added) and gains a new "secrets/credential masking (Phase 12)" section. `CHANGELOG`'s Phase 12 entry no longer overstates the `[ingestion-secrets]` extra: the regex set is the sole detection backend in v0.5.2, and the `detect-secrets` package is reserved for a follow-up release. `README` separates "From PyPI" and "From a local clone" install blocks so copy-paste users don't hit `-e .` confusion. - **Test fixtures fragmented** — All hardcoded credential / JWT literals in `tests/test_data_audit_phase12.py`, `tests/test_ingestion_phase12.py`, and `tests/test_phase12_review_fixes.py` now built at runtime from inert string fragments (e.g. `"AKIA" + "IOSFODNN7" + "EXAMPLE"`). The regex still has to match the canonical shape, but no full literal credential lives in the source tree — silences gitleaks / trufflehog scans of the repo without changing behaviour. - **5 new round-2 regression tests** (`tests/test_phase12_review_fixes.py`) — `TestTildeFenceRecognised` (~~~-fenced code blocks block heading splits), `TestPrivateKeyFullBlock` (full PEM body redaction), `TestMarkdownOverlapValidation` (rejection on explicit non-zero overlap; default-overlap pass-through), `TestMinHashDistinctSemantic` (unique-sketches semantic). - **Notebook ruff format** — `notebooks/post_training_workflow.ipynb` reformatted to satisfy `ruff format --check` in CI; `notebooks/data_curation.ipynb` install line pinned to `forgelm[ingestion]==0.5.2` rather than the moving `main` branch. @@ -1141,7 +1283,7 @@ Round-1 review of the Phase 12 commit surfaced four critical regressions / bugs ### Added — Phase 12 (Data Curation Maturity, targeting v0.5.2) -Direct continuation of the Phase 11 / 11.5 ingestion + audit lineage. Closes the four concrete gaps surfaced by the post-`v0.5.1` competitive review (LLaMA-Factory / Axolotl / Unsloth / NeMo Curator / Dolma / RedPajama / LlamaIndex / LangChain / Marker / Docling). Tier 1 (5 must-have tasks) shipped; Tier 2/3 (Presidio adapter, Croissant metadata, `--all-mask`, wizard "audit first") deferred to a [Phase 12.5 backlog](docs/roadmap/phase-12-5-backlog.md). +Direct continuation of the Phase 11 / 11.5 ingestion + audit lineage. Closes the four concrete gaps surfaced by the post-`v0.5.1` competitive review (LLaMA-Factory / Axolotl / Unsloth / NeMo Curator / Dolma / RedPajama / LlamaIndex / LangChain / Marker / Docling). Tier 1 (5 must-have tasks) shipped; Tier 2/3 (Presidio adapter, Croissant metadata, `--all-mask`, wizard "audit first") deferred to a [Phase 12.5 backlog](docs/roadmap/completed-phases.md). - **MinHash LSH dedup option** (`forgelm/data_audit.py`: `compute_minhash`, `find_near_duplicates_minhash`, `_count_leaked_rows_minhash`) — Opt-in `--dedup-method minhash --jaccard-threshold 0.85` route via the optional `[ingestion-scale]` extra (`datasketch>=1.6.0`). Default simhash + LSH banding from Phase 11.5 stays untouched and remains the only method that runs without an optional dependency. `audit_dataset(...)` API gains `dedup_method`, `minhash_jaccard`, `minhash_num_perm` parameters; `near_duplicate_summary.method` records which path ran. Cross-split overlap + within-split duplicate scan share the same method flag. MinHash is approximate (permutation noise; default `num_perm=128`) — pin `num_perm` for cross-run determinism. - **Markdown-aware splitter** (`forgelm/ingestion.py`: `_chunk_markdown`, `_chunk_markdown_tokens`, `_markdown_sections`, `_heading_breadcrumb`) — New `--strategy markdown` parses heading hierarchy (`# H1` … `###### H6`), keeps code-fenced blocks atomic (heading-shaped lines inside ```` ``` ```` blocks are not interpreted as section boundaries), and inlines a heading **breadcrumb** at the top of each chunk so SFT loss sees the document context. Composes with the Phase 11.5 token-aware mode (`--chunk-tokens` + `--tokenizer`). @@ -1305,7 +1447,7 @@ Operational polish on top of `v0.5.0`'s ingestion + audit surface — no new tra ### Documentation - New "Option 0: One-Command Quickstart Template" section at the top of `docs/guides/quickstart.md`. -- `docs/roadmap.md`, `docs/roadmap-tr.md`, `docs/roadmap/phase-10-5-quickstart.md`, `docs/roadmap/releases.md` updated to mark Phase 10.5 as Done. +- `docs/roadmap.md`, `docs/roadmap-tr.md`, `docs/roadmap/completed-phases.md`, `docs/roadmap/releases.md` updated to mark Phase 10.5 as Done. - `README.md` quickstart section updated to lead with `forgelm quickstart`. --- diff --git a/CLAUDE.md b/CLAUDE.md index 48fa566d..61f86fea 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -154,10 +154,11 @@ Default workflow for a non-trivial change: python3 tools/check_anchor_resolution.py --strict && \ python3 tools/check_cli_help_consistency.py --strict && \ python3 tools/check_wizard_defaults_sync.py && \ - python3 tools/check_no_analysis_refs.py + python3 tools/check_no_analysis_refs.py && \ + python3 tools/check_no_unguarded_sys_modules_pop.py ``` - All nine must pass. The first four are the historical gauntlet; + All ten must pass. The first four are the historical gauntlet; the three doc guards (Wave 3 / Wave 4 / Wave 5 additions) catch bilingual structural drift, broken markdown anchors, and CLI ↔ docs help-text drift before the PR opens. The wizard-defaults guard @@ -166,6 +167,10 @@ Default workflow for a non-trivial change: (review-cycle 5) keeps the public tree from citing gitignored `docs/marketing/` or `docs/analysis/` paths — see `docs/standards/documentation.md` "Working-memory directories". + The unguarded-`sys.modules`-pop guard (v0.5.7 round-4) flags any + `sys.modules.pop("torch"|"numpy"|"trl"|…)` without + `monkeypatch.delitem` — the v0.5.7 round-3 review traced 35 + spurious full-suite failures to that exact pattern. ## Etiquette when communicating with the user diff --git a/docs/roadmap-tr.md b/docs/roadmap-tr.md index 4c53e3fa..a5e4bbda 100644 --- a/docs/roadmap-tr.md +++ b/docs/roadmap-tr.md @@ -7,10 +7,10 @@ | Tür | Faz | Durum | |-----|-----|-------| | ✅ Tamam | [Faz 1-9](roadmap/completed-phases.md) | SOTA iyileştirmeleri, değerlendirme, güvenilirlik, kurumsal entegrasyon, ekosistem, hizalama stack'i, güvenlik, EU AI Act uyumluluğu (Madde 9-17 + Ek IV), gelişmiş güvenlik zekası | -| ✅ Tamam | [Faz 10 — Post-Training Tamamlama](roadmap/phase-10-post-training.md) | `inference.py`, `chat`, `export` (GGUF), `--fit-check`, `deploy` — `v0.4.0` | -| ✅ Tamam | [Faz 10.5 — Quickstart Katmanı ve Onboarding](roadmap/phase-10-5-quickstart.md) | `forgelm quickstart