diff --git a/CHANGELOG.md b/CHANGELOG.md index 6a4d5500..f521bb23 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,9 +4,36 @@ 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. +Review-absorption rounds 3 + 4 + 5 follow-up to v0.5.7. No public +surface change, but training-pipeline UX, doctor robustness, and +test-isolation guarantees improve. Round 5 closes the residual +CodeRabbit findings against the round-3/4 commits. + +### Fixed (round 5 — CodeRabbit follow-up) + +- **`forgelm/cli/_training.py::_preflight_numpy_torch_abi`** — any + unexpected exception from the underlying probe (corrupted torch + install where `torch.__version__` raises `AttributeError`, etc.) + is now caught and converted into a structured + `abi_preflight_crashed` JSON envelope. Previously the raw Python + traceback would pre-empt the `--output-format json` contract that + every other CLI failure path honours. Exit code stays + `EXIT_TRAINING_ERROR` (= 2), matching the broken-ABI verdict so + CI/CD branching doesn't need to distinguish "ABI bad" from "ABI + probe died". +- **`CLAUDE.md`** — exit-code contract was stated as `0/1/2/3/4`, + but the canonical table in `docs/standards/error-handling.md` + documented `0/1/2/3/4/5` since v0.5.5 (Phase 22 added + `EXIT_WIZARD_CANCELLED = 5`). CLAUDE.md now matches the actual + contract — the standard was right, the agent guidance was stale. +- **`docs/roadmap/completed-phases.md::Phase 12` summary** — Tier 1 + status line claimed a `[ingestion-secrets]` extra "via + `detect-secrets` with regex fallback", but that extra was never + published; only `[ingestion-pii-ml]` exists in `pyproject.toml`'s + extras surface. Reworded to record the historical plan accurately + ("`detect-secrets` integration was originally planned as a + `[ingestion-secrets]` extra but deferred — only + `[ingestion-pii-ml]` ultimately shipped"). Pure docs accuracy fix. ### Added diff --git a/CLAUDE.md b/CLAUDE.md index 61f86fea..99b2feae 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -104,7 +104,7 @@ These come from the standards documents; summarized here for quick reference: 1. **Config-driven.** Behaviour is determined by validated YAML. No env-var sniffing for behaviour (only for secrets). No hardcoded feature flags. 2. **Reliability before features.** Every new capability ships with tests, docs, and CI coverage. "I'll add tests later" = the PR is not ready. 3. **Optional dependencies as extras.** Heavy deps (`bitsandbytes`, `unsloth`, `deepspeed`, `lm-eval`, `wandb`, `mergekit`) live under `[project.optional-dependencies]` and raise `ImportError` with an install hint when missing. -4. **Exit codes are a public contract.** 0/1/2/3/4 — see [error-handling.md](docs/standards/error-handling.md). CI/CD pipelines depend on these. +4. **Exit codes are a public contract.** 0/1/2/3/4/5 — see [error-handling.md](docs/standards/error-handling.md) for the full table (`0=success`, `1=config`, `2=training`, `3=eval-failure`, `4=awaiting-approval`, `5=wizard-cancelled`). CI/CD pipelines depend on these. 5. **Append-only audit log.** Every decision gate emits a structured event. Never edit or delete entries. 6. **No silent failures.** No bare `except:`, no `except Exception: pass`, no `|| true` in CI, no logging-and-swallowing for anything except explicitly-non-fatal paths (webhooks, cleanup). 7. **Bilingual where it counts.** User-facing docs are EN + TR mirrors. Code, CLI output, logs, config keys are English only. diff --git a/docs/roadmap/completed-phases.md b/docs/roadmap/completed-phases.md index 2789ac3a..df6534b0 100644 --- a/docs/roadmap/completed-phases.md +++ b/docs/roadmap/completed-phases.md @@ -923,7 +923,7 @@ suite because it is wall-clock-noisy. > and 12.5 — see [releases.md](releases.md#v050-document-ingestion-data-curation-pipeline). > Version-label references below preserve the historical planning trail. > -> **Status:** ✅ **Tier 1 DONE** — landed on `development` for the (originally) `v0.5.2` cycle. All five must-have tasks shipped: MinHash LSH dedup option (`[ingestion-scale]` extra via `datasketch`), markdown-aware splitter (`--strategy markdown`), code/secrets leakage tagger (`[ingestion-secrets]` extra via `detect-secrets` with regex fallback), heuristic quality filter (`--quality-filter`), DOCX/Markdown table preservation. Tier 2 (Presidio adapter, Croissant metadata) and Tier 3 (`--all-mask` composite, wizard "audit first" hook) are deferred to a follow-up **Phase 12.5** backlog file (analogous to [Phase 11.5 section above](#phase-115-ingestion-audit-polish)). Modules: [`forgelm/data_audit.py`](../../forgelm/data_audit/), [`forgelm/ingestion.py`](../../forgelm/ingestion.py), [`forgelm/cli.py`](../../forgelm/cli/); tests: [`tests/test_data_audit_phase12.py`](../../tests/test_data_audit_phase12.py), [`tests/test_ingestion_phase12.py`](../../tests/test_ingestion_phase12.py); CLI tests added in [`tests/test_cli_subcommands.py`](../../tests/test_cli_subcommands.py). +> **Status:** ✅ **Tier 1 DONE** — landed on `development` for the (originally) `v0.5.2` cycle. All five must-have tasks shipped: MinHash LSH dedup option (`[ingestion-scale]` extra via `datasketch`), markdown-aware splitter (`--strategy markdown`), code/secrets leakage tagger (regex-based; `detect-secrets` integration was originally planned as a `[ingestion-secrets]` extra but deferred — only `[ingestion-pii-ml]` for Presidio NER ultimately shipped in `pyproject.toml`), heuristic quality filter (`--quality-filter`), DOCX/Markdown table preservation. Tier 2 (Presidio adapter, Croissant metadata) and Tier 3 (`--all-mask` composite, wizard "audit first" hook) are deferred to a follow-up **Phase 12.5** backlog file (analogous to [Phase 11.5 section above](#phase-115-ingestion-audit-polish)). Modules: [`forgelm/data_audit.py`](../../forgelm/data_audit/), [`forgelm/ingestion.py`](../../forgelm/ingestion.py), [`forgelm/cli.py`](../../forgelm/cli/); tests: [`tests/test_data_audit_phase12.py`](../../tests/test_data_audit_phase12.py), [`tests/test_ingestion_phase12.py`](../../tests/test_ingestion_phase12.py); CLI tests added in [`tests/test_cli_subcommands.py`](../../tests/test_cli_subcommands.py). > **Note:** Phase 11 + 11.5 built the ingestion / audit lineage (Phase 11 → `v0.5.0`, Phase 11.5 → `v0.5.1`); this phase moves the same lineage from **enterprise-acceptable** to **enterprise-competitive**. diff --git a/forgelm/cli/_training.py b/forgelm/cli/_training.py index ab470c01..016423aa 100644 --- a/forgelm/cli/_training.py +++ b/forgelm/cli/_training.py @@ -46,10 +46,46 @@ def _preflight_numpy_torch_abi(json_output: bool) -> None: command if the mismatch is present. No-op on healthy platforms (Linux, Apple Silicon, fresh-install Intel Mac), so zero false positives. + + Any unexpected exception from the probe itself (a corrupted torch + install where ``torch.__version__`` raises ``AttributeError``, for + instance) converts into a structured ``abi_preflight_crashed`` + envelope so the operator never sees a raw Python traceback + pre-empt the JSON contract — matching the rest of the CLI's + "every failure path carries a JSON envelope" rule per + ``docs/standards/error-handling.md``. """ from ._abi_check import ABI_BROKEN, compute_numpy_torch_abi_status, format_abi_remediation - status, torch_version, numpy_version = compute_numpy_torch_abi_status() + try: + status, torch_version, numpy_version = compute_numpy_torch_abi_status() + except Exception as e: # noqa: BLE001 — see docstring above: preflight + # is a sentinel that must NEVER let an unexpected exception escape + # to the user as a raw Python traceback. CodeRabbit round-5 + # absorption: wrap the probe so the JSON envelope contract holds + # even when the underlying torch / numpy install is corrupted + # enough that ``compute_numpy_torch_abi_status`` itself raises. + _report_training_error( + json_output, + payload={ + "success": False, + "error": "abi_preflight_crashed", + "exception_class": type(e).__name__, + "exception_message": str(e), + "remediation": ( + "The torch/NumPy ABI preflight crashed unexpectedly. This " + "usually indicates a corrupted torch or numpy install. Run " + "`forgelm doctor` for the full environment diagnostic." + ), + }, + log_msg=( + f"ABI preflight crashed unexpectedly ({type(e).__name__}: {e}). Run `forgelm doctor` for diagnostics." + ), + exit_code=EXIT_TRAINING_ERROR, + with_traceback=True, + ) + return + if status != ABI_BROKEN: return remediation = format_abi_remediation(torch_version, numpy_version) diff --git a/tests/test_abi_check.py b/tests/test_abi_check.py index e14e00bc..0648af06 100644 --- a/tests/test_abi_check.py +++ b/tests/test_abi_check.py @@ -209,3 +209,45 @@ def test_no_op_for(self, status): result = (status, "2.4.0", "1.26.4") with patch.object(_abi_check, "compute_numpy_torch_abi_status", return_value=result): _training._preflight_numpy_torch_abi(json_output=False) + + +class TestPreflightHandlesProbeCrash: + """A crash in ``compute_numpy_torch_abi_status`` itself (corrupted + torch where ``torch.__version__`` raises, theoretical numpy ABI + poisoning that breaks the import-time machinery, etc.) must be + converted into a structured exit so the operator never sees a + raw Python traceback pre-empt the JSON envelope contract. + + Round-5 absorption of CodeRabbit's MAJOR finding against the + round-3/4 preflight wiring.""" + + def _exploding_status(self): + raise AttributeError("module 'torch' has no attribute '__version__'") + + def test_crash_exits_with_training_error_code(self, caplog): + with patch.object(_abi_check, "compute_numpy_torch_abi_status", side_effect=self._exploding_status): + with caplog.at_level("ERROR"), pytest.raises(SystemExit) as exc_info: + _training._preflight_numpy_torch_abi(json_output=False) + # Exit-code contract: the preflight crash maps to + # EXIT_TRAINING_ERROR (= 2 in the public table) — same class + # as the broken-ABI verdict, so CI/CD doesn't need to + # distinguish "ABI bad" from "ABI probe died". + assert exc_info.value.code == 2 + # Operator-visible log must mention the actionable next step. + assert "forgelm doctor" in caplog.text + assert "ABI preflight crashed" in caplog.text + + def test_crash_emits_structured_json_envelope(self, capsys): + with patch.object(_abi_check, "compute_numpy_torch_abi_status", side_effect=self._exploding_status): + with pytest.raises(SystemExit): + _training._preflight_numpy_torch_abi(json_output=True) + captured = capsys.readouterr() + envelope = json.loads(captured.out) + # Lock the key set — CI/CD consumers that parse this path + # need a stable shape, distinct from the "numpy_torch_abi_ + # mismatch" envelope so they can branch on root cause. + assert envelope["success"] is False + assert envelope["error"] == "abi_preflight_crashed" + assert envelope["exception_class"] == "AttributeError" + assert "__version__" in envelope["exception_message"] + assert "forgelm doctor" in envelope["remediation"]