diff --git a/.claude/skills/cut-release/SKILL.md b/.claude/skills/cut-release/SKILL.md index 0828967d..bdf8dd1a 100644 --- a/.claude/skills/cut-release/SKILL.md +++ b/.claude/skills/cut-release/SKILL.md @@ -174,7 +174,8 @@ If the install fails or returns the wrong version, investigate the workflow run. ### 1. Announce -Template in [docs/marketing/05_content_strategy.md](../../../docs/marketing/05_content_strategy.md) under "New Release": +Use this template (the maintainer keeps a longer canonical version in +local working memory; the spirit is the same): ``` 📦 ForgeLM v0.4.0 released! @@ -205,7 +206,10 @@ git push ### 3. Update marketing roadmap metrics -[`docs/marketing/marketing_strategy_roadmap.md`](../../../docs/marketing/marketing_strategy_roadmap.md) — metrics row for the release month. +The maintainer keeps a marketing roadmap in local working memory +(gitignored). Add a metrics row for the release month there as part +of the release-day checklist; nothing on the public surface needs to +change. ### 4. Close the phase (if applicable) diff --git a/.claude/skills/sync-bilingual-docs/SKILL.md b/.claude/skills/sync-bilingual-docs/SKILL.md index 9dc0f611..cde9b2e5 100644 --- a/.claude/skills/sync-bilingual-docs/SKILL.md +++ b/.claude/skills/sync-bilingual-docs/SKILL.md @@ -17,12 +17,15 @@ Any change to these paths or their siblings: Do **not** use for: - `docs/standards/*` (English only) -- `docs/marketing/*` (local-only, mixed) -- `docs/analysis/*` (whichever language suits the source) +- `docs/marketing/*` (gitignored working-memory; mixed languages) - `docs/qms/*` (English only) - `docs/design/*` (English only) - `docs/guides/*` (English only for now — Turkish is future work) +Do **not** reference any file under gitignored working-memory directories +(`docs/marketing/`, `docs/analysis/`) from the bilingual docs you edit — +those paths are local-only and won't resolve in fresh clones. + ## Required reading 1. [docs/standards/localization.md](../../../docs/standards/localization.md) — the full policy diff --git a/.codacy.yaml b/.codacy.yaml new file mode 100644 index 00000000..c3e81c9e --- /dev/null +++ b/.codacy.yaml @@ -0,0 +1,24 @@ +# Codacy configuration — exclusion paths for generated artefacts. +# +# The exclusions below pin two categories that produce false-positive +# Codacy findings for this project: +# +# 1. ``site/js/wizard_defaults.js`` is a build-time artefact emitted by +# ``tools/generate_wizard_defaults.py`` from Pydantic field defaults +# flagged with ``json_schema_extra={"wizard": True}`` (F1 review-cycle 3). +# PMD's ``InaccurateNumericLiteral`` rule flags scientific notation +# like ``2e-05``; the value comes from Python's ``json.dumps`` which +# chooses scientific notation for small floats — and the JS consumer +# canonicalises with ``Number.toExponential()`` so precision is never +# operator-visible. The CI guard +# ``tools/check_wizard_defaults_sync.py`` already verifies the +# generated content matches the schema byte-for-byte; secondary +# static analysis adds no signal. +# +# 2. ``forgelm/wizard/_defaults.json`` — same generator output for the +# Python consumer. Plain JSON, no analysis applies. +# +# Codacy's analysis YAML format mirrors GitHub Actions' path globs. +exclude_paths: + - site/js/wizard_defaults.js + - forgelm/wizard/_defaults.json diff --git a/.gitignore b/.gitignore index 5d837dd9..f8c2b4cd 100644 --- a/.gitignore +++ b/.gitignore @@ -1,33 +1,9 @@ -# Internal strategy, marketing, and analysis docs (not public) +# Gitignored working-memory directories — local-only PR-cycle audit +# notes, external-repo comparisons, drafts. See +# ``docs/standards/documentation.md`` ("Working-memory directories") +# for the rule: no public-doc / production-code references allowed. docs/marketing/ docs/analysis/ -# Explicit exceptions: a small set of v0.5.5-closure planning artifacts -# is exposed publicly (referenced from closure PRs and the roadmap). The -# rest of docs/analysis/** stays internal — including all subdirectories -# (QKV-Core, Trion, autoresearch, proposals, ...) and every other file -# under code_reviews/. -# -# Pattern explanation: -# 1. docs/analysis/ — ignore everything under this dir -# 2. !docs/analysis/ — re-allow the directory itself (so we can descend) -# 3. docs/analysis/* — ignore every direct entry (subdirs + files) -# 4. !docs/analysis/code_reviews/ — re-allow code_reviews/ subdir only -# 5. docs/analysis/code_reviews/* — ignore every entry in code_reviews/ -# 6. ! — re-include the 3 closure planning artifacts -!docs/analysis/ -docs/analysis/* -!docs/analysis/code_reviews/ -docs/analysis/code_reviews/* -!docs/analysis/code_reviews/closure-plan-202604300906.md -!docs/analysis/code_reviews/master-review-opus-202604300906.md -!docs/analysis/code_reviews/split-design-data_audit-cli-202604300906.md -!docs/analysis/code_reviews/wave2b-final-merge-review-prompt.md -!docs/analysis/code_reviews/wave3-review-prompt.md -!docs/analysis/code_reviews/wave3-followup-review-prompt.md -!docs/analysis/code_reviews/iso27001-soc2-alignment-202605052315.md -!docs/analysis/code_reviews/wave4-review-prompt.md -!docs/analysis/code_reviews/wave4-followup-review-prompt.md -!docs/analysis/code_reviews/wave5-review-prompt.md # Generated by tools/build_usermanuals.py — produced from # docs/usermanuals//**/*.md during the CI deploy. Not committed; diff --git a/.markdownlint.json b/.markdownlint.json index 54e5902d..b410e052 100644 --- a/.markdownlint.json +++ b/.markdownlint.json @@ -2,7 +2,9 @@ "//": "ForgeLM markdownlint policy. Consumed by Codacy's markdownlint scanner (loadbearing — without this config Codacy emits MD051 across every SPA hash-router link in docs/usermanuals/).", "//1": "MD051 (link-fragments-valid): docs use SPA hash-router routes like `#/data/audit` for in-app navigation; markdownlint's anchor-resolution doesn't understand SPA conventions and would flag every cross-reference.", "//2": "MD014 (commands-show-output): shell examples in operator docs deliberately show only the command (`$ forgelm audit ...`) without sample output — output snippets rot quickly across versions and would force a doc update on every CLI banner change.", + "//3": "MD013 (line-length): bilingual (EN+TR) reference + design docs intentionally carry long lines for paragraph-style explanations, table cells, and prose with long inline links. The 80-col limit is a code-style holdover that hurts readability of documentation; the project's bilingual_parity guard already enforces structural alignment, and the rest of the prose stays human-readable at any reasonable width.", "default": true, "MD051": false, - "MD014": false + "MD014": false, + "MD013": false } diff --git a/CHANGELOG.md b/CHANGELOG.md index 192f8688..8960c83c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,14 +4,562 @@ All notable changes to ForgeLM are documented here. ## [Unreleased] +### Changed + +- **Working-memory directories cleanup (review-cycle 5 / 2026-05-09).** + ``docs/marketing/`` and ``docs/analysis/`` are now strictly gitignored + with **zero exceptions**, and the public tree no longer cites them. + Closes a long-running documentation-drift surface where review notes, + audit drafts, and external-repo comparisons under those directories + were referenced from CHANGELOG entries, design docs, standards, and + test docstrings — only to rot into 404s the moment the maintainer + touched the local working-memory tree. + - **`.gitignore`** simplified: dropped 11 explicit "re-include" + exception entries that re-exposed specific closure-plan and + master-review files; the directory-level ignore now applies + uniformly. + - **`git rm --cached -r docs/analysis/`** untracked the 11 + previously-leaked files (kept on disk locally; just no longer + versioned). + - **19 references** removed / restructured across `CHANGELOG.md`, + `CLAUDE.md`, `docs/standards/{coding,documentation,localization}.md`, + `docs/design/iso27001_soc2_alignment.md`, `docs/roadmap.md`, + `docs/roadmap/phase-12-6-closure-cycle.md`, + `docs/roadmap/phase-12-data-curation-maturity.md`, + `tests/test_wizard_phase22.py`, + `tests/test_check_bilingual_parity.py`, + `tools/check_anchor_resolution.py`, + `tools/check_yaml_snippets.py`, + `.claude/skills/{cut-release,sync-bilingual-docs}/SKILL.md`. + Where the citation was the load-bearing anchor of a sentence, + the surrounding text was rewritten to convey the same intent + without the file reference. + - **New "Working-memory directories" rule** in + `docs/standards/documentation.md` codifies the ban: no public-tree + file (markdown, code, commit message, PR description) may link + into `docs/marketing/` or `docs/analysis/`. Path-string filters + in production code (e.g. `_SKIP_PATH_FRAGMENTS`) are exempt as + functional path exclusions, not content references. + - **New CI guard `tools/check_no_analysis_refs.py`** scans every + git-tracked file (uses `git ls-files`) and fails the run on any + citation into the working-memory tree. Wired into the self- + review chain in `CLAUDE.md` (now 9 commands). False positives + are handled via an `_EXEMPT` allowlist with per-entry + justification comments. + - **Memory section in `CLAUDE.md`** rewritten so the two + working-memory directories are described together with the new + "no references allowed" rule, replacing the prior bullet that + framed `docs/analysis/` as citable research. + - **Site verification report (read-only)** generated as part of + the same audit pass: 3 CRITICAL + 5 HIGH + 6 MEDIUM + 3 LOW + findings on `site/` claims vs. live code. The report itself + lives in the now-gitignored `docs/analysis/` working-memory + tree (per the new policy); follow-up site fixes are tracked + as separate PRs. + +### Fixed + +- **Wizard PR-D contract violations — idempotent re-run actually + preserves prior answers (PR-E / 2026-05-09).** The independent + review of PR-D (`2ac9df2`) found three HIGH-severity bugs where + pressing Enter at certain prompts silently regressed the operator's + loaded YAML to wizard defaults, plus four MEDIUM/LOW issues. All + closed in this commit. + - **PR-D-A1: ``_step_strategy`` now honours existing values** + for ``lora.method`` / ``lora.target_modules`` / ``lora.dropout`` + / ``lora.bias`` / ``lora.task_type``. ``_select_strategy()`` + accepts ``existing_method`` / ``existing_load_in_4bit`` / + ``existing_galore`` kwargs and derives the prompt's ``default_idx`` + so a bare Enter on a DoRA / PiSSA / rsLoRA / GaLore config keeps + the choice. Target-modules prompt detects the loaded list shape + against ``TARGET_MODULE_PRESETS`` and shifts its default + accordingly. Non-prompted fields (dropout / bias / task_type) now + use ``setdefault`` so existing operator overrides survive. + - **PR-D-A2: ``_step_evaluation`` no longer clobbers existing + benchmark / llm_judge / safety blocks.** The function now seeds + ``evaluation`` from ``state.config.get("evaluation")`` via + ``copy.deepcopy``, defaults the ``auto_revert`` gate based on the + existing value, and keeps the prior block intact when the + operator declines a re-prompt. The pre-fix code built a fresh + dict and assigned it unconditionally — high-risk operators with + populated benchmark + judge configs lost them on a "press Enter" + iteration. + - **PR-D-A3: ``_step_use_case`` skips the use-case prompt when + existing model + trainer choices are detected.** Pre-fix the + operator's ``model.name_or_path`` and ``training.trainer_type`` + were silently overwritten by the first template's preset (default + index = 1, "customer-support") on Enter. Combined with the + save-default-to-``start_from`` behaviour, this corrupted the + original YAML. Step now emits a clear "Existing model / trainer + choices detected — skipping use-case preset" line and early- + returns when state already carries those keys. Even on greenfield + runs, ``setdefault`` is now used so explicit values survive. + - **PR-D-A4: ``_step_welcome`` ``model.backend`` chained + ``setdefault``** instead of nested-dict-replace pattern. An + operator with an explicit ``model.backend: transformers`` on a + Linux+GPU box no longer flips silently to ``unsloth`` based on + hardware detection. + - **PR-D-A5: ``_load_initial_state_from_yaml`` split try/except** + so a downstream ``ImportError`` from inside a Pydantic validator + can't silently bypass schema validation. The ``ForgeConfig`` + import lives in its own try block; the ``model_validate`` call is + wrapped separately. + - **PR-D-A6: warns before discarding pre-existing wizard resume + state.** When ``--wizard-start-from`` is supplied alongside an + in-progress ``$XDG_CACHE_HOME/forgelm/wizard_state.yaml``, the + operator now sees a one-line notice that the snapshot will be + cleared at save time. Pre-fix the snapshot was silently + destroyed at the end of the start-from run. + - **PR-D-A7: tightened ``docs/guides/quickstart{,-tr}.md`` claim** + so it accurately scopes "press Enter to keep" to numeric / text / + choice prompts (now), explicitly notes the use-case skip, and + records the ``setdefault`` pattern for non-prompted fields. + - **PR-D-B1: new CLI dispatcher integration test** + (``test_wizard_start_from_threads_through_to_run_wizard_full``) + pins the parser → dispatcher → ``run_wizard_full`` thread-through + so a future rename of the argparse ``dest`` would be caught. + - **PR-D-B3: ``--wizard-start-from`` without ``--wizard``** now + prints a one-line warning instead of silently no-op'ing. + - **PR-D-B5: ``_collect_data_governance(existing=...)``** kwarg + threads the operator's existing Article 10 free-text values + through as prompt defaults. Strict-tier operators iterating + from a populated YAML no longer re-type collection_method / + annotation_process / known_biases. + - **PR-D-B6: ``_canonical_start_from()`` helper** applies + ``Path(...).expanduser()`` once at the entry point and + propagates the canonical form through the load + save flow. + Pre-fix, ``--wizard-start-from ~/configs/x.yaml`` would expand + ``~`` for the load existence check but pass the raw string to + the save flow's overwrite confirmation, causing the check to + return False and a literal ``~/configs/x.yaml`` directory to be + created on save. + - **15 new regression tests** under ``tests/test_wizard_phase22.py``: + DoRA preserved, target_modules extended preserved, GaLore + preserved, evaluation benchmark + judge preserved, use-case skip, + backend setdefault, ImportError split happy path, resume-state + warning, --start-from-without-wizard, governance defaults, and + expanduser canonicalisation. Plus 1 dispatcher integration test. + ### Added +- **Wizard idempotent re-run via ``--wizard-start-from `` + (E3 / PR-D / 2026-05-09).** Operators can now iterate on an + existing config without losing prior answers — long-asked-for + ergonomic improvement that closes the "wizard always starts from + scratch" gap noted across review cycles 1, 2, and 3. Bound to F1 + (PR-B) since the per-step value-honoring relies on the same + schema-derived defaults plumbing. + - **New CLI flag:** ``forgelm --wizard --wizard-start-from + /path/to/existing.yaml`` reads the YAML, validates it against + ``ForgeConfig`` (immediate failure on schema violation rather + than 30 minutes into a failed training run), and seeds the + wizard's :class:`_WizardState.config` with the loaded dict. + The quickstart-template prelude is skipped — the operator's + intent is "edit this file", not "pick a fresh template". + - **Per-step value honoring:** ``_step_strategy`` / + ``_step_training_params`` / ``_step_dataset`` now read existing + values from ``state.config`` for their prompt defaults so a bare + Enter keeps them; ``_step_dataset`` adds an explicit "keep + existing dataset?" yes/no early-out so the operator doesn't + re-trigger ingestion / audit on an unchanged dataset path. + - **Save flow defaults to start-from path:** when launched with + ``--wizard-start-from existing.yaml`` the save prompt defaults + to the same path; the existing overwrite confirmation + (``_prompt_unique_filename``) still fires before clobbering. + - **New ``_load_initial_state_from_yaml(path)`` helper** in + ``forgelm/wizard/_orchestrator.py``. Surfaces three distinct + error categories: missing path → ``FileNotFoundError``; + parse failure → ``ValueError`` with YAML diagnostic; non-mapping + root or schema rejection → ``ValueError`` with a clear hint. + All three are caught by ``run_wizard_full`` and surfaced as a + cancelled outcome with a printed warning instead of a traceback. + - **10 new tests** cover: valid-YAML pre-population (with + deepcopy isolation), missing-path error, malformed YAML, non- + mapping root, schema-invalid YAML, dispatcher cancel-on-error, + per-step value-honoring for strategy / training-params / dataset. + +- **Wizard review-cycle 3 follow-up — generator hardening + CI guard + test coverage (2026-05-09).** Closes the actionable findings from + the independent review of commits `5667885` (cycle-3 bug fixes) + + `91d5726` (F1 SOT). + - **B1: generator unwraps ``Optional[BaseModel]`` sub-blocks** + (`tools/generate_wizard_defaults.py`). Pre-fix the walk only + matched bare-class annotations, so a wizard flag on a field + inside an `Optional[XxxConfig]` sub-model was silently skipped. + 10 of 14 ForgeConfig top-level fields are `Optional[...]` — + today no flags exist on them, but a future contributor would + have hit the silent-skip trap. New `_unwrap_basemodel()` helper + walks `typing.get_args` to find the inner BaseModel. + - **B2: PydanticUndefined identity comparison.** Replaces the + brittle `repr(default).startswith("PydanticUndefined")` with the + canonical `default is PydanticUndefined` import from + `pydantic_core`. Stable across Pydantic v2 minor releases. + - **C2: generator JSON-serialisable contract documented** as a + module-level header. Future flags on Path / datetime / callable + defaults fail loudly at generator run, but the contract is now + explicit so contributors don't get surprised. + - **B5: schema↔fallback drift WARNING in `_state.py`.** When the + shipped JSON and the hardcoded fallback disagree, the wizard now + emits a `logger.warning` so the second-source-of-truth drift + surfaces in CI logs. Happy path is silent (matches → no warning). + - **G2: CI guard fail-path test coverage.** New + `tests/test_check_wizard_defaults_sync.py` with 7 tests: + corrupt-JSON triggers failure + diff message, missing-target + triggers failure, diff truncation works, generator unwraps + Optional[BaseModel], required fields skipped, etc. Without this + a regression in the diff-printing logic could silently report + "OK" against a corrupted JSON. + - **G3: dispatcher test now asserts `ForgeConfig.model_validate` + on the test fixture YAML.** Pre-fix, a future change to + `minimal_config()` that produced an invalid YAML would still + pass `test_wizard_start_training_routes_through_dispatcher` + (because the trainer pipeline is mocked before validation). + - **B7: `defer` attribute on `wizard_defaults.js`.** Both wizard + scripts now defer; HTML5 spec guarantees execution order + (document order) so `WIZARD_DEFAULTS` still lands before + `defaultState()` runs, but neither blocks HTML parsing. + - **A3: hardware-cache freshness caveat documented** in + `_cached_hardware()` docstring (GPU hot-plug mid-session is an + acceptable edge case). + - **F3: README directory tree** mentions `_defaults.json` shipped + as schema-derived defaults SOT. + - **CI guard self-protection: `_display_path()` helper** in + `check_wizard_defaults_sync.py` falls back to absolute path when + the target is outside `_REPO_ROOT` (prevents `Path.relative_to` + crash when tests monkeypatch the target to a tmp dir). + +- **Wizard schema-driven defaults SOT (F1 / 2026-05-09).** Closes the + long-running drift surface tracked through P1-P17 across review + cycles 1, 2, and 3. A schema field default change without a + matching wizard update was the single biggest source of CLI ↔ web + parity gaps; this PR turns the schema into the canonical source. + - **Pydantic field metadata flag:** wizard-relevant fields in + `forgelm/config.py` carry `json_schema_extra={"wizard": True}` + (8 fields today: `model.max_length`, `lora.r/alpha/dropout`, + `training.num_train_epochs/per_device_train_batch_size/learning_rate/gradient_accumulation_steps`). + - **Generator script:** `tools/generate_wizard_defaults.py` walks + every nested submodel reachable from `ForgeConfig`, extracts the + flagged defaults, and writes two artefacts: + - `forgelm/wizard/_defaults.json` — package data shipped via + `[tool.setuptools.package-data]` so wheel installs preserve it. + - `site/js/wizard_defaults.js` — tiny script that exposes + `window.WIZARD_DEFAULTS` for the web wizard's `defaultState()`. + - **CI guard:** `tools/check_wizard_defaults_sync.py` re-runs the + generator into a temp comparison and fails the run if the + shipped artefacts drift from the schema. Operators get a clear + "regenerate via …" hint with the first 5 differing lines. + - **Python consumer:** `forgelm/wizard/_state.py` now reads + `DEFAULT_*` constants from the shipped JSON via + `importlib.resources`. Hardcoded fallbacks survive only when the + JSON is missing entirely (broken pip install). + - **JS consumer:** `site/js/wizard.js` `defaultState()` reads + schema-derived values via a new `wd(section, key, fallback)` + helper; ` + + diff --git a/tests/conftest.py b/tests/conftest.py index 44a62f88..509e715c 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -41,3 +41,27 @@ def _pin_audit_operator(monkeypatch): via ``monkeypatch.delenv`` inside the test body. """ monkeypatch.setenv("FORGELM_OPERATOR", os.environ.get("FORGELM_OPERATOR") or "test-operator") + + +@pytest.fixture(autouse=True) +def _isolate_wizard_state(request, tmp_path_factory, monkeypatch): + """B10 — keep wizard XDG state out of the developer's real ``~/.cache``. + + Any test under ``tests/test_wizard_*`` (or anything that ends up + invoking ``forgelm.wizard._save_wizard_state`` indirectly) writes + a YAML to ``$XDG_CACHE_HOME/forgelm/wizard_state.yaml``. Without + isolation a contributor running ``pytest`` would have their real + in-flight wizard snapshot clobbered. + + We redirect ``XDG_CACHE_HOME`` to a per-test tmp dir for every + wizard-flavoured test file. The ``test_wizard_phase22`` module + already has its own ``isolated_state_dir`` fixture; this one is + additive — they coexist because they both monkeypatch the same + env var, and the more-specific fixture takes precedence on the + tests that explicitly request it. + """ + test_path = str(getattr(request.node, "fspath", "") or "") + if "test_wizard_" not in test_path and "test_phase12_5" not in test_path: + return + isolated = tmp_path_factory.mktemp("wizard_xdg_isolated") + monkeypatch.setenv("XDG_CACHE_HOME", str(isolated)) diff --git a/tests/test_check_bilingual_parity.py b/tests/test_check_bilingual_parity.py index e0fe10cb..e2e6a81f 100644 --- a/tests/test_check_bilingual_parity.py +++ b/tests/test_check_bilingual_parity.py @@ -296,8 +296,9 @@ def test_every_tr_mirror_appears_in_pair_registry(self) -> None: Out-of-scope: ``docs/usermanuals/`` has its own structural validator (``tools/build_usermanuals.py``); top-level docs like ``docs/roadmap-tr.md`` and ``docs/product_strategy-tr.md`` - diverge intentionally; ``docs/analysis/`` is internal / - gitignored research. + diverge intentionally; gitignored working-memory directories + (``docs/marketing/``, ``docs/analysis/``) are excluded by + construction. Allowlist: ``safety_compliance-tr.md`` carries a known in-progress structural drift (34 H2/H3/H4 deltas as of Wave diff --git a/tests/test_check_wizard_defaults_sync.py b/tests/test_check_wizard_defaults_sync.py new file mode 100644 index 00000000..73b26ae2 --- /dev/null +++ b/tests/test_check_wizard_defaults_sync.py @@ -0,0 +1,158 @@ +"""Tests for ``tools/check_wizard_defaults_sync.py`` (G2 / review-cycle 3). + +The CI guard's happy path is implicitly covered by the gauntlet's +``python tools/check_wizard_defaults_sync.py`` invocation in +``CLAUDE.md``. This module pins the FAIL path: when the shipped +artefacts disagree with a fresh schema regeneration, ``main()`` must +exit non-zero AND surface a meaningful diff. + +Without this test a regression in the diff-printing logic (or in the +byte-comparison branch) could cause the guard to silently report +"OK" against a corrupted JSON, producing exactly the schema↔shipped +drift the guard is meant to catch. + +The guard also exercises ``tools/generate_wizard_defaults.py``'s +new B1 (Optional[BaseModel] unwrap) + B2 (PydanticUndefined identity +comparison) hardening as a side effect — the regenerated artefacts +must be byte-identical to the committed ones, which only works if +the walk produces deterministic output. +""" + +from __future__ import annotations + +import sys +from pathlib import Path + +import pytest + +_REPO_ROOT = Path(__file__).resolve().parent.parent +if str(_REPO_ROOT) not in sys.path: + sys.path.insert(0, str(_REPO_ROOT)) + + +class TestCIGuardFailPath: + """The guard must exit non-zero + show a diff when artefacts drift.""" + + def test_corrupt_json_triggers_failure(self, tmp_path, monkeypatch, capsys): + from tools import check_wizard_defaults_sync as guard + + # Redirect the guard's PYTHON_TARGET to a tmp file containing + # deliberately wrong JSON; the JS_TARGET stays pointed at the + # real (correct) shipped file so we can verify the guard + # discovers the drift on Python side specifically. + bad_python = tmp_path / "_defaults.json" + bad_python.write_text( + '{\n "lora": {\n "r": 99\n }\n}\n', # wrong: schema says r=8 + encoding="utf-8", + ) + monkeypatch.setattr(guard, "PYTHON_TARGET", bad_python) + rc = guard.main() + assert rc != 0, "guard must exit non-zero on schema↔JSON drift" + out = capsys.readouterr().out + assert "drifted" in out, "diff message must call out the drift" + # The guard prints "Fix: … python tools/generate_wizard_defaults.py" + # so the operator knows how to repair without scanning docs. + assert "tools/generate_wizard_defaults.py" in out + + def test_missing_target_triggers_failure(self, tmp_path, monkeypatch, capsys): + from tools import check_wizard_defaults_sync as guard + + # Point at a non-existent file; the guard must NOT crash and + # MUST report the missing-file failure clearly. + nonexistent = tmp_path / "_defaults.json" # never created + monkeypatch.setattr(guard, "PYTHON_TARGET", nonexistent) + rc = guard.main() + assert rc != 0 + out = capsys.readouterr().out + assert "missing" in out.lower() + + def test_diff_output_truncated_at_5_lines(self, tmp_path, monkeypatch, capsys): + from tools import check_wizard_defaults_sync as guard + + # Force MANY differing lines to verify the truncation ellipsis fires. + bad_lines = "\n".join(f' "wrong_key_{i}": {i}' for i in range(20)) + bad_python = tmp_path / "_defaults.json" + bad_python.write_text("{\n" + bad_lines + "\n}\n", encoding="utf-8") + monkeypatch.setattr(guard, "PYTHON_TARGET", bad_python) + rc = guard.main() + assert rc != 0 + out = capsys.readouterr().out + # Truncation marker is part of the contract — operators should + # not be flooded with thousands of diff lines. + assert "truncated" in out.lower() + + +class TestGeneratorOptionalUnwrap: + """B1 — generator unwraps ``Optional[BaseModel]`` so flagged sub-fields aren't lost.""" + + def test_unwrap_basemodel_handles_optional(self): + # Construct a synthetic Optional[BaseModel] annotation and + # assert _unwrap_basemodel returns the inner class. Avoids + # depending on which ForgeConfig sub-blocks are Optional today. + from typing import Optional + + from pydantic import BaseModel + + from tools.generate_wizard_defaults import _unwrap_basemodel + + class _Inner(BaseModel): + pass + + assert _unwrap_basemodel(_Inner) is _Inner + assert _unwrap_basemodel(Optional[_Inner]) is _Inner + + def test_unwrap_basemodel_returns_none_for_unrelated_types(self): + from tools.generate_wizard_defaults import _unwrap_basemodel + + assert _unwrap_basemodel(int) is None + assert _unwrap_basemodel(str) is None + + def test_walk_picks_up_flag_on_optional_submodel(self): + # Synthetic schema where the Optional[Inner] sub-block carries + # a wizard-flagged field; pre-B1 the walk would silently skip + # this entirely. + from typing import Optional + + from pydantic import BaseModel, Field + + from tools.generate_wizard_defaults import _walk_model + + class _Inner(BaseModel): + magic: int = Field(default=42, json_schema_extra={"wizard": True}) + + class _Outer(BaseModel): + inner: Optional[_Inner] = None + + sink = {} + # Drive the walk from _Outer's perspective the same way + # ``collect_defaults`` does at the top level. + from tools.generate_wizard_defaults import _unwrap_basemodel + + for name, field_info in _Outer.model_fields.items(): + nested = _unwrap_basemodel(field_info.annotation) + if nested is not None: + _walk_model(nested, name, sink) + assert sink == {"inner": {"magic": 42}}, "Optional[Inner] sub-block was silently skipped — B1 regression" + + +class TestGeneratorPydanticUndefinedIdentity: + """B2 — required fields detected via ``is PydanticUndefined``, not repr().""" + + def test_required_wizard_flagged_field_skipped(self): + from pydantic import BaseModel, Field + + from tools.generate_wizard_defaults import _walk_model + + class _Schema(BaseModel): + # No default — Pydantic stores PydanticUndefined as the sentinel. + required_thing: str = Field(json_schema_extra={"wizard": True}) + + sink = {} + _walk_model(_Schema, "test", sink) + # Required fields must not appear in the output (no default to + # serialise); the walk should skip them via identity check. + assert sink == {}, "Required field with no default leaked into output" + + +if __name__ == "__main__": + sys.exit(pytest.main([__file__, "-v"])) diff --git a/tests/test_cli_phase10.py b/tests/test_cli_phase10.py index f20c281d..cff4b575 100644 --- a/tests/test_cli_phase10.py +++ b/tests/test_cli_phase10.py @@ -369,10 +369,105 @@ def test_existing_flags_unchanged_after_subcommand_addition(self, tmp_path): assert exc_info.value.code == EXIT_SUCCESS def test_wizard_still_works(self): - """--wizard flow must be unaffected.""" - mock_wizard = MagicMock(return_value=None) - with patch("forgelm.wizard.run_wizard", mock_wizard): + """--wizard flow must remain reachable via the dispatcher. + + Post-D2 (review-cycle 2 / 2026-05-09) the dispatcher consumes + ``run_wizard_full`` and emits ``EXIT_SUCCESS`` when the operator + produced + deferred (saved a YAML but answered "no" to "start + training now?"). ``EXIT_WIZARD_CANCELLED = 5`` is now the + exit code for genuine cancels (Ctrl-C, non-tty refusal, + decline-to-save) — covered separately below. + """ + from forgelm.wizard._orchestrator import WizardOutcome + + deferred = WizardOutcome(config_path="/tmp/saved.yaml", start_training=False) + with patch("forgelm.wizard.run_wizard_full", MagicMock(return_value=deferred)): with patch("sys.argv", ["forgelm", "--wizard"]): with pytest.raises(SystemExit) as exc_info: main() assert exc_info.value.code == EXIT_SUCCESS + + def test_wizard_cancelled_exits_5(self): + """D2: a cancelled wizard exits ``EXIT_WIZARD_CANCELLED`` (5).""" + from forgelm.cli._exit_codes import EXIT_WIZARD_CANCELLED + from forgelm.wizard._orchestrator import WizardOutcome + + cancelled = WizardOutcome(config_path=None, start_training=False) + with patch("forgelm.wizard.run_wizard_full", MagicMock(return_value=cancelled)): + with patch("sys.argv", ["forgelm", "--wizard"]): + with pytest.raises(SystemExit) as exc_info: + main() + assert exc_info.value.code == EXIT_WIZARD_CANCELLED + + def test_wizard_start_from_threads_through_to_run_wizard_full(self, monkeypatch): + """PR-D-B1 (PR-E review fix): the --wizard-start-from flag must reach + run_wizard_full as the start_from kwarg. + + Pre-fix coverage stopped at the orchestrator boundary; a future + rename of the argparse ``dest`` would silently regress to + ``None`` (legacy behaviour) without any test catching it. This + test stubs ``run_wizard_full`` to capture its kwargs, builds an + argparse Namespace via the real parser, and asserts the path + flowed through parser → dispatcher → wizard. + """ + from forgelm.wizard._orchestrator import WizardOutcome + + captured: dict = {} + + def _stub_run_wizard_full(*, start_from=None): + captured["start_from"] = start_from + return WizardOutcome(config_path=None, start_training=False) + + with patch("forgelm.wizard.run_wizard_full", _stub_run_wizard_full): + with patch("sys.argv", ["forgelm", "--wizard", "--wizard-start-from", "/tmp/some-config.yaml"]): + with pytest.raises(SystemExit): + main() + assert captured["start_from"] == "/tmp/some-config.yaml", ( + "--wizard-start-from did not thread through to run_wizard_full" + ) + + def test_wizard_start_training_routes_through_dispatcher(self, monkeypatch, tmp_path, minimal_config): + """E22-24 (review-cycle 3): the start_training=True branch must mutate + ``args.config`` and let the trainer pipeline take over. + + The pre-cycle test only exercised the deferred + cancel paths; + the dispatcher's ``args.config = outcome.config_path`` line was + uncovered. Stub the trainer so the test stays GPU-free + fast. + """ + import yaml as _yaml + + from forgelm.wizard._orchestrator import WizardOutcome + + cfg_path = tmp_path / "wizard.yaml" + config_data = minimal_config() + cfg_path.write_text(_yaml.safe_dump(config_data), encoding="utf-8") + # G3 (review-cycle 3): defensive hardening — verify the YAML + # the test writes actually loads cleanly. Without this assert, + # a future change to ``minimal_config`` that produced an + # invalid YAML would still pass this test (because the + # trainer pipeline is mocked BEFORE validation runs in + # ``_dispatch.main``), giving false-positive coverage. + from forgelm.config import ForgeConfig + + ForgeConfig.model_validate(config_data) + outcome = WizardOutcome(config_path=str(cfg_path), start_training=True) + + captured = {} + + # ``_run_training_pipeline`` is called as + # ``_run_training_pipeline(config, args, json_output)`` from + # ``forgelm/cli/_dispatch.py:232``. Stub captures the args + # object so the test can verify ``args.config`` was mutated. + def _stub_pipeline(_config_obj, args, *_a, **_kw): + captured["config"] = args.config + sys.exit(EXIT_SUCCESS) + + with patch("forgelm.wizard.run_wizard_full", MagicMock(return_value=outcome)): + monkeypatch.setattr("forgelm.cli._dispatch._run_training_pipeline", _stub_pipeline) + with patch("sys.argv", ["forgelm", "--wizard"]): + with pytest.raises(SystemExit) as exc_info: + main() + # The dispatcher must have set args.config = outcome.config_path + # and routed into the (stubbed) trainer pipeline. + assert captured["config"] == str(cfg_path) + assert exc_info.value.code == EXIT_SUCCESS diff --git a/tests/test_phase12_5.py b/tests/test_phase12_5.py index 418b181e..344a4f83 100644 --- a/tests/test_phase12_5.py +++ b/tests/test_phase12_5.py @@ -125,7 +125,7 @@ def test_audit_offer_runs_audit_when_user_accepts(self, tmp_path, capsys): self._write_jsonl(ds, [{"text": "alpha"}, {"text": "beta"}]) # Simulate "yes" to the offer. - with patch("forgelm.wizard._prompt_yes_no", return_value=True): + with patch("forgelm.wizard._byod._prompt_yes_no", return_value=True): outcome = _offer_audit_for_jsonl(ds) assert outcome is True @@ -139,7 +139,7 @@ def test_audit_offer_skipped_when_user_declines(self, tmp_path, capsys): self._write_jsonl(ds, [{"text": "alpha"}]) # Simulate "no" — must short-circuit before invoking audit_dataset. - with patch("forgelm.wizard._prompt_yes_no", return_value=False): + with patch("forgelm.wizard._byod._prompt_yes_no", return_value=False): with patch("forgelm.data_audit.audit_dataset") as mock_audit: outcome = _offer_audit_for_jsonl(ds) mock_audit.assert_not_called() @@ -155,7 +155,7 @@ def test_audit_offer_recovers_when_audit_raises(self, tmp_path, capsys): ds = tmp_path / "data.jsonl" self._write_jsonl(ds, [{"text": "alpha"}]) - with patch("forgelm.wizard._prompt_yes_no", return_value=True): + with patch("forgelm.wizard._byod._prompt_yes_no", return_value=True): with patch( "forgelm.data_audit.audit_dataset", side_effect=RuntimeError("simulated audit failure"), diff --git a/tests/test_wizard_phase11.py b/tests/test_wizard_phase11.py index bd4b0a46..bb667e04 100644 --- a/tests/test_wizard_phase11.py +++ b/tests/test_wizard_phase11.py @@ -244,19 +244,27 @@ def test_collect_rope_scaling_short_context_silent(self, max_length, expect_none assert "Long context detected" not in captured def test_print_wizard_summary_includes_strategy_and_dataset(self, capsys): + # Phase 22 rewrite: ``_print_wizard_summary`` takes the resolved + # config dict instead of ~12 keyword arguments. The summary + # body now also includes the full YAML preview (G17), so we + # only assert on the labelled summary lines that the rest of + # the wizard depends on. wizard._print_wizard_summary( - model_name="meta-llama/Llama-3.1-8B-Instruct", - suggested_backend="transformers", - use_galore=False, - load_in_4bit=True, - use_dora=False, - trainer_type="sft", - lora_r=16, - lora_alpha=32, - dataset_path="tatsu-lab/alpaca", - epochs=3, - batch_size=4, - output_dir="./checkpoints", + { + "model": { + "name_or_path": "meta-llama/Llama-3.1-8B-Instruct", + "backend": "transformers", + "load_in_4bit": True, + }, + "lora": {"r": 16, "alpha": 32, "method": "lora"}, + "training": { + "trainer_type": "sft", + "num_train_epochs": 3, + "per_device_train_batch_size": 4, + "output_dir": "./checkpoints", + }, + "data": {"dataset_name_or_path": "tatsu-lab/alpaca"}, + } ) out = capsys.readouterr().out assert "Configuration Summary" in out @@ -271,38 +279,37 @@ def test_print_wizard_summary_galore_strategy(self, capsys): # branch so a reordering of the if/elif ladder can't silently # swallow the GaLore label in summaries. wizard._print_wizard_summary( - model_name="m", - suggested_backend="transformers", - use_galore=True, - load_in_4bit=False, - use_dora=False, - trainer_type="sft", - lora_r=8, - lora_alpha=16, - dataset_path="d", - epochs=1, - batch_size=1, - output_dir="./out", + { + "model": {"name_or_path": "m", "backend": "transformers", "load_in_4bit": False}, + "lora": {"r": 8, "alpha": 16, "method": "lora"}, + "training": { + "trainer_type": "sft", + "galore_enabled": True, + "num_train_epochs": 1, + "per_device_train_batch_size": 1, + "output_dir": "./out", + }, + "data": {"dataset_name_or_path": "d"}, + } ) assert "Strategy: GaLore" in capsys.readouterr().out def test_print_wizard_summary_dora_suffix(self, capsys): wizard._print_wizard_summary( - model_name="m", - suggested_backend="transformers", - use_galore=False, - load_in_4bit=True, - use_dora=True, - trainer_type="dpo", - lora_r=8, - lora_alpha=16, - dataset_path="d", - epochs=1, - batch_size=1, - output_dir="./out", + { + "model": {"name_or_path": "m", "backend": "transformers", "load_in_4bit": True}, + "lora": {"r": 8, "alpha": 16, "method": "dora"}, + "training": { + "trainer_type": "dpo", + "num_train_epochs": 1, + "per_device_train_batch_size": 1, + "output_dir": "./out", + }, + "data": {"dataset_name_or_path": "d"}, + } ) out = capsys.readouterr().out - assert "Strategy: QLoRA + DoRA" in out + assert "Strategy: QLoRA + DORA" in out assert "Trainer: DPO" in out diff --git a/tests/test_wizard_phase22.py b/tests/test_wizard_phase22.py new file mode 100644 index 00000000..dd09610c --- /dev/null +++ b/tests/test_wizard_phase22.py @@ -0,0 +1,1759 @@ +"""Phase 22 / 2026-05-08: CLI wizard parity-with-web modernisation tests. + +Covers the new helpers + step-machine plumbing introduced when the CLI +wizard was extended to close the parity gap with ``site/js/wizard.js`` +(20 findings G1-G20 + 5 independent observations I1-I5 documented in +the PR #40 review thread): + +- ``_parse_webhook_value`` — single-prompt webhook syntax with + ``env:VAR_NAME`` prefix and HTTPS validation. +- ``_default_safety_probes_path`` — package-data resolution of the + bundled probe set. +- ``_collect_trainer_hyperparameters`` — per-trainer fields gating. +- ``_check_navigation_token`` — back / reset sentinels. +- ``_apply_strict_tier_coercion`` — F-compliance-110 front-stop. +- ``_save_wizard_state`` / ``_load_wizard_state`` / + ``_clear_wizard_state`` — XDG-aware persistence. +- ``_print_step_diff`` — terminal-friendly state diff. + +No GPU / no network — every test stubs ``builtins.input`` for stdin +reads and uses a temp directory for persistence. +""" + +from __future__ import annotations + +from pathlib import Path +from unittest.mock import patch + +import pytest + +from forgelm import wizard + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + + +def _input_returning(*answers): + """Build a side_effect for ``builtins.input`` that drains *answers*.""" + queue = list(answers) + + def _impl(_prompt_text=""): + if not queue: + raise AssertionError("input() called more times than answers provided") + return queue.pop(0) + + return _impl + + +# --------------------------------------------------------------------------- +# Webhook URL parsing — Phase 22 / G15 +# --------------------------------------------------------------------------- + + +class TestParseWebhookValue: + def test_empty_returns_none(self): + assert wizard._parse_webhook_value("") is None + assert wizard._parse_webhook_value(" ") is None + + def test_https_url_accepted(self): + section = wizard._parse_webhook_value("https://hooks.slack.com/services/T/B/X") + assert section == {"url": "https://hooks.slack.com/services/T/B/X"} + + def test_env_prefix_emits_url_env(self): + section = wizard._parse_webhook_value("env:SLACK_WEBHOOK_URL") + assert section == {"url_env": "SLACK_WEBHOOK_URL"} + + def test_env_prefix_case_insensitive_keyword(self): + # The ``env:`` keyword itself is matched case-insensitively so + # operators copying values from the web wizard's YAML aren't + # tripped up by case. + section = wizard._parse_webhook_value("ENV:MY_WEBHOOK") + assert section == {"url_env": "MY_WEBHOOK"} + + def test_env_prefix_empty_var_name_rejected(self): + with pytest.raises(ValueError, match="non-empty variable name"): + wizard._parse_webhook_value("env:") + + def test_env_prefix_lowercase_var_name_rejected(self): + # POSIX env-var convention enforces uppercase letters; lower- + # case names break ``${VAR}`` shell expansion in CI. + with pytest.raises(ValueError, match="POSIX environment-variable name"): + wizard._parse_webhook_value("env:lowercase_var") + + def test_bare_string_no_scheme_rejected(self): + with pytest.raises(ValueError, match="not a valid URL"): + wizard._parse_webhook_value("not-a-url") + + def test_unknown_scheme_rejected(self): + with pytest.raises(ValueError, match="must use `https://`"): + wizard._parse_webhook_value("ftp://example.com/hook") + + def test_http_warning_but_accepted(self, capsys): + section = wizard._parse_webhook_value("http://hooks.example.com/x") + assert section == {"url": "http://hooks.example.com/x"} + captured = capsys.readouterr().out + assert "uses HTTP, not HTTPS" in captured + + +# --------------------------------------------------------------------------- +# Safety probe path resolution — Phase 22 / G16 +# --------------------------------------------------------------------------- + + +class TestDefaultSafetyProbesPath: + def test_resolves_to_packaged_jsonl(self): + # The bundled probe set lives at + # ``forgelm/safety_prompts/default_probes.jsonl`` and is + # shipped via ``[tool.setuptools.package-data]`` in + # ``pyproject.toml`` (Phase 36). The resolver must produce a + # real on-disk path the trainer can ``open``. + path = wizard._default_safety_probes_path() + assert path.endswith("default_probes.jsonl") + assert Path(path).is_file() + + def test_path_is_absolute_or_resolvable(self): + # ``pkgutil.get_data`` would return bytes; the wizard prefers + # ``importlib.resources.files`` so the operator gets a string + # path they can hand to ``forgelm safety-eval --probes``. + path = wizard._default_safety_probes_path() + assert Path(path).resolve().is_file() + + +# --------------------------------------------------------------------------- +# Navigation tokens — Phase 22 / G3 +# --------------------------------------------------------------------------- + + +class TestNavigationTokens: + def test_back_token_raises_wizardback(self): + for token in ("back", "b", "BACK", " Back "): + with pytest.raises(wizard.WizardBack): + wizard._check_navigation_token(token) + + def test_reset_token_raises_wizardreset(self): + for token in ("reset", "r", "RESET", " Reset "): + with pytest.raises(wizard.WizardReset): + wizard._check_navigation_token(token) + + def test_cancel_token_does_not_raise(self): + # Cancel is contextual — the BYOD path interprets it as "fall + # back to the full wizard" and the step orchestrator relies on + # Ctrl-C / Ctrl-D for clean exits. Auto-raising on cancel + # would break the existing BYOD flow. + for token in ("cancel", "c", "q", "quit"): + wizard._check_navigation_token(token) + + def test_empty_string_does_not_raise(self): + wizard._check_navigation_token("") + wizard._check_navigation_token(" ") + + def test_normal_input_does_not_raise(self): + wizard._check_navigation_token("Llama-3.1-8B") + wizard._check_navigation_token("yes") + wizard._check_navigation_token("3") + + +# --------------------------------------------------------------------------- +# Trainer-specific hyperparameters — Phase 22 / G1 +# --------------------------------------------------------------------------- + + +class TestCollectTrainerHyperparameters: + def test_sft_returns_empty(self): + # SFT has no per-trainer knobs in ``TrainingConfig``. + with patch("builtins.input", side_effect=_input_returning()): + result = wizard._collect_trainer_hyperparameters("sft") + assert result == {} + + def test_dpo_returns_dpo_beta(self): + with patch("builtins.input", side_effect=_input_returning("0.1")): + result = wizard._collect_trainer_hyperparameters("dpo") + assert result == {"dpo_beta": 0.1} + + def test_simpo_returns_beta_and_gamma(self): + with patch("builtins.input", side_effect=_input_returning("2.0", "0.5")): + result = wizard._collect_trainer_hyperparameters("simpo") + assert result == {"simpo_beta": 2.0, "simpo_gamma": 0.5} + + def test_kto_returns_kto_beta(self): + with patch("builtins.input", side_effect=_input_returning("0.1")): + result = wizard._collect_trainer_hyperparameters("kto") + assert result == {"kto_beta": 0.1} + + def test_orpo_returns_orpo_beta(self): + with patch("builtins.input", side_effect=_input_returning("0.1")): + result = wizard._collect_trainer_hyperparameters("orpo") + assert result == {"orpo_beta": 0.1} + + def test_grpo_returns_full_block_with_reward_model(self): + with patch( + "builtins.input", + side_effect=_input_returning("4", "512", "my_reward.score"), + ): + result = wizard._collect_trainer_hyperparameters("grpo") + assert result == { + "grpo_num_generations": 4, + "grpo_max_completion_length": 512, + "grpo_reward_model": "my_reward.score", + } + + def test_grpo_omits_reward_model_when_blank(self): + # Empty reward_model means "use the built-in shaper" — which + # the schema represents as ``None``. The wizard drops the key + # entirely so the YAML doesn't carry an empty string the + # validator would have to special-case. + with patch("builtins.input", side_effect=_input_returning("4", "512", "")): + result = wizard._collect_trainer_hyperparameters("grpo") + assert result == { + "grpo_num_generations": 4, + "grpo_max_completion_length": 512, + } + assert "grpo_reward_model" not in result + + +# --------------------------------------------------------------------------- +# Strict-tier auto-coercion — Phase 22 / G8 +# --------------------------------------------------------------------------- + + +class TestApplyStrictTierCoercion: + def test_minimal_risk_no_op(self): + config = {"evaluation": {"auto_revert": True}} + wizard._apply_strict_tier_coercion(config, {"risk_classification": "minimal-risk"}) + # Should NOT have added safety + require_human_approval. + assert "safety" not in config["evaluation"] + assert "require_human_approval" not in config["evaluation"] + + def test_high_risk_enables_safety_and_human_approval(self, capsys): + config: dict = {} + wizard._apply_strict_tier_coercion(config, {"risk_classification": "high-risk"}) + assert config["evaluation"]["require_human_approval"] is True + assert config["evaluation"]["safety"]["enabled"] is True + assert config["evaluation"]["auto_revert"] is True + captured = capsys.readouterr().out + assert "Article 9" in captured + assert "Article 14" in captured + + def test_unacceptable_also_triggers_coercion(self): + config: dict = {} + wizard._apply_strict_tier_coercion(config, {"risk_classification": "unacceptable"}) + assert config["evaluation"]["safety"]["enabled"] is True + + def test_does_not_overwrite_explicit_safety_block(self): + # Operator already configured safety with track_categories=False; + # coercion mustn't clobber their choices when safety is already + # enabled. + config = {"evaluation": {"safety": {"enabled": True, "track_categories": False}}} + wizard._apply_strict_tier_coercion(config, {"risk_classification": "high-risk"}) + assert config["evaluation"]["safety"]["enabled"] is True + assert config["evaluation"]["safety"]["track_categories"] is False + + +# --------------------------------------------------------------------------- +# Persistence — Phase 22 / G6 +# --------------------------------------------------------------------------- + + +@pytest.fixture +def isolated_state_dir(tmp_path, monkeypatch): + """Redirect the wizard state directory to a tmp_path.""" + monkeypatch.setenv("XDG_CACHE_HOME", str(tmp_path)) + yield tmp_path / "forgelm" + + +class TestPersistence: + def test_load_returns_none_when_no_snapshot(self, isolated_state_dir): + assert wizard._load_wizard_state() is None + + def test_save_and_load_roundtrip(self, isolated_state_dir): + snapshot = { + "experience": "beginner", + "use_case": "customer-support", + "current_step": 3, + "completed_steps": ["welcome", "use-case", "model"], + "config": {"model": {"name_or_path": "Qwen/Qwen2.5-7B-Instruct"}}, + } + wizard._save_wizard_state(snapshot) + loaded = wizard._load_wizard_state() + assert loaded == snapshot + + def test_clear_removes_snapshot(self, isolated_state_dir): + wizard._save_wizard_state({"experience": "expert"}) + wizard._clear_wizard_state() + assert wizard._load_wizard_state() is None + + def test_clear_is_idempotent(self, isolated_state_dir): + # Clearing a non-existent snapshot must not raise — the wizard + # calls ``_clear_wizard_state`` on successful completion even + # when nothing was saved. + wizard._clear_wizard_state() + wizard._clear_wizard_state() # still fine + + def test_load_returns_none_on_version_mismatch(self, isolated_state_dir): + path = wizard._wizard_state_path() + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text("v: 99999\nexperience: expert\n", encoding="utf-8") + assert wizard._load_wizard_state() is None + + def test_load_returns_none_on_corrupt_snapshot(self, isolated_state_dir): + path = wizard._wizard_state_path() + path.parent.mkdir(parents=True, exist_ok=True) + # Invalid YAML — must not raise; just silently miss. + path.write_text("not: valid: yaml: at: all", encoding="utf-8") + assert wizard._load_wizard_state() is None + + def test_state_path_honours_xdg_cache_home(self, tmp_path, monkeypatch): + monkeypatch.setenv("XDG_CACHE_HOME", str(tmp_path)) + assert wizard._wizard_state_path() == tmp_path / "forgelm" / "wizard_state.yaml" + + def test_state_path_falls_back_to_home_cache_when_xdg_unset(self, monkeypatch): + monkeypatch.delenv("XDG_CACHE_HOME", raising=False) + path = wizard._wizard_state_path() + # ``Path.home() / ".cache"`` may differ across CI environments, + # but the trailing components are stable. + assert path.name == "wizard_state.yaml" + assert path.parent.name == "forgelm" + + +# --------------------------------------------------------------------------- +# Step-diff preview — Phase 22 / G7 +# --------------------------------------------------------------------------- + + +class TestStepDiff: + def test_no_diff_emits_nothing(self, capsys): + wizard._print_step_diff({"a": 1}, {"a": 1}, "step1") + assert capsys.readouterr().out == "" + + def test_added_keys_marked_with_plus(self, capsys): + wizard._print_step_diff({}, {"model": {"name_or_path": "X"}}, "model") + out = capsys.readouterr().out + assert "Step diff (model):" in out + assert "+ model.name_or_path: 'X'" in out + + def test_changed_keys_marked_with_tilde(self, capsys): + wizard._print_step_diff({"a": 1}, {"a": 2}, "params") + out = capsys.readouterr().out + assert "~ a: 1 → 2" in out + + def test_nested_dict_paths_dotted(self, capsys): + wizard._print_step_diff( + {"training": {"epochs": 3}}, + {"training": {"epochs": 3, "batch_size": 4}}, + "training-params", + ) + out = capsys.readouterr().out + assert "+ training.batch_size: 4" in out + assert "training.epochs" not in out # unchanged → not printed + + +# --------------------------------------------------------------------------- +# Webhook collector — Phase 22 / G15 (integration around ``_parse_webhook_value``) +# --------------------------------------------------------------------------- + + +class TestCollectWebhookConfig: + def test_decline_returns_none(self): + with patch("builtins.input", side_effect=_input_returning("n")): + assert wizard._collect_webhook_config() is None + + def test_env_prefix_default_accepted(self): + # The prompt default is ``env:FORGELM_WEBHOOK_URL`` so an empty + # answer accepts the suggested env-var. ``notify_on_start`` + # defaults to ``False`` (web-wizard parity) — start + # notifications are noisy and most operators want only success + # / failure pings. + with patch("builtins.input", side_effect=_input_returning("y", "")): + section = wizard._collect_webhook_config() + assert section == { + "url_env": "FORGELM_WEBHOOK_URL", + "notify_on_start": False, + "notify_on_success": True, + "notify_on_failure": True, + } + + def test_https_url_accepted(self): + with patch( + "builtins.input", + side_effect=_input_returning("y", "https://hooks.slack.com/x"), + ): + section = wizard._collect_webhook_config() + assert section is not None + assert section["url"] == "https://hooks.slack.com/x" + + def test_invalid_url_reprompts(self, capsys): + # First answer is malformed; second is a valid env: ref. + with patch( + "builtins.input", + side_effect=_input_returning("y", "not-a-url", "env:MY_HOOK"), + ): + section = wizard._collect_webhook_config() + assert section is not None + assert section["url_env"] == "MY_HOOK" + captured = capsys.readouterr().out + assert "not a valid URL" in captured + + +# --------------------------------------------------------------------------- +# Use-case preset registry — Phase 22 / G12 + I4 +# --------------------------------------------------------------------------- + + +class TestUseCasePresets: + def test_presets_match_quickstart_template_keys(self): + # I4: the web wizard's USE_CASE_PRESETS keys must adopt the + # CLI quickstart TEMPLATES keys (single source of truth). This + # asserts the CLI side of the agreement: every TEMPLATES key + # appears in ``_wizard_use_case_presets``. + from forgelm.quickstart import TEMPLATES + + presets = wizard._wizard_use_case_presets() + for key in TEMPLATES.keys(): + assert key in presets, f"Use-case preset for '{key}' missing" + + def test_custom_preset_present(self): + # ``custom`` is a wizard-only escape hatch that doesn't seed + # any defaults. + presets = wizard._wizard_use_case_presets() + assert wizard._MANUAL_USE_CASE in presets + assert presets[wizard._MANUAL_USE_CASE]["model"] is None + + +# --------------------------------------------------------------------------- +# Strategy choice mapping — Phase 22 / G2 +# --------------------------------------------------------------------------- + + +class TestStrategyChoices: + def test_six_strategies_listed(self): + # QLoRA / LoRA / DoRA / PiSSA / rsLoRA / GaLore — the full + # ``LoraConfigModel.method`` Literal plus GaLore as a separate + # axis. + assert len(wizard._STRATEGY_CHOICES) == 6 + + def test_methods_cover_full_lora_literal(self): + methods = {choice.method for choice in wizard._STRATEGY_CHOICES if not choice.use_galore} + assert methods == {"lora", "dora", "pissa", "rslora"} + + def test_galore_choice_is_singleton(self): + galore_choices = [c for c in wizard._STRATEGY_CHOICES if c.use_galore] + assert len(galore_choices) == 1 + + +# --------------------------------------------------------------------------- +# GaLore optimizer variants — Phase 22 / G9 +# --------------------------------------------------------------------------- + + +class TestGaloreOptimizers: + def test_six_variants_listed(self): + assert len(wizard._GALORE_OPTIMIZERS) == 6 + + def test_includes_layerwise_siblings(self): + # The original wizard listed only the three base variants. + # Phase 22 surfaces all six, including ``_layerwise`` siblings + # that drop peak VRAM further by recomputing per-layer. + assert "galore_adamw_layerwise" in wizard._GALORE_OPTIMIZERS + assert "galore_adamw_8bit_layerwise" in wizard._GALORE_OPTIMIZERS + assert "galore_adafactor_layerwise" in wizard._GALORE_OPTIMIZERS + + +# --------------------------------------------------------------------------- +# Schema-default parity — Phase 22 / G11 +# --------------------------------------------------------------------------- + + +class TestSchemaDefaultParity: + def test_lora_r_default_matches_schema(self): + # G11: the v0.5.5 wizard set DEFAULT_LORA_R = 16 while the + # schema default in ``LoraConfigModel.r`` was 8. Phase 22 + # aligns the wizard with the schema so an operator who accepts + # every prompt produces a YAML byte-equivalent to + # ``ForgeConfig()``. + from forgelm.config import LoraConfigModel + + assert wizard.DEFAULT_LORA_R == LoraConfigModel.model_fields["r"].default + + def test_lora_alpha_default_is_2x_r(self): + # Adapter convention: alpha = 2 * r. + assert wizard.DEFAULT_LORA_ALPHA == 2 * wizard.DEFAULT_LORA_R + + def test_default_epochs_matches_schema(self): + from forgelm.config import TrainingConfig + + assert wizard.DEFAULT_EPOCHS == TrainingConfig.model_fields["num_train_epochs"].default + + def test_default_batch_size_matches_schema(self): + from forgelm.config import TrainingConfig + + assert wizard.DEFAULT_BATCH_SIZE == TrainingConfig.model_fields["per_device_train_batch_size"].default + + def test_default_lr_matches_schema(self): + from forgelm.config import TrainingConfig + + assert wizard.DEFAULT_LR == TrainingConfig.model_fields["learning_rate"].default + + +# --------------------------------------------------------------------------- +# Orchestrator step-machine — Phase 22 / G3 + G7 + I3 +# +# The orchestrator was the lowest-coverage module after the Phase 22 +# split (14 % per the post-merge review). These tests exercise the +# step driver directly with stub steps so we don't need to mock 9 +# real ``_collect_*`` helpers. +# --------------------------------------------------------------------------- + + +class TestStepMachineDriver: + """Exercise ``_drive_wizard_steps`` directly with stub steps.""" + + def _make_steps(self, runners): + """Build a tuple of ``_StepDef``-shaped objects for *runners*.""" + return tuple( + wizard._orchestrator._StepDef(label=f"step-{i}", runner=runner) for i, runner in enumerate(runners) + ) + + def test_runs_every_step_in_order(self, isolated_state_dir, monkeypatch): + order = [] + + def make(label): + def runner(state): + order.append(label) + state.config[label] = True + + return runner + + steps = self._make_steps([make("a"), make("b"), make("c")]) + monkeypatch.setattr(wizard._orchestrator, "_STEPS", steps) + + state = wizard._orchestrator._drive_wizard_steps(wizard._WizardState()) + assert order == ["a", "b", "c"] + assert state.completed_steps == ["step-0", "step-1", "step-2"] + assert state.current_step == 3 + + def test_back_restores_prev_config(self, isolated_state_dir, monkeypatch): + # Step 1 mutates state.config; step 2 mutates it then raises + # WizardBack. Mutations from step 2 must NOT leak back into + # step 1 — that's the whole point of the prev_config snapshot. + attempts = {"step-1": 0} + + def step0(state): + state.config.setdefault("model", {})["name"] = "from-step-0" + + def step1_first(state): + # First entry: mutate then back out. + attempts["step-1"] += 1 + if attempts["step-1"] == 1: + state.config["leaked"] = "should-not-survive" + raise wizard.WizardBack + + def step0_after_back(state): + # Re-running step 0 — the leaked key from step 1's first + # attempt MUST be gone. + assert "leaked" not in state.config + + # Stitch: step 0 runs twice (first then after back); step 1 + # runs once (raises back), then once more silently. + runs = {"step-0": 0} + + def step0_combined(state): + runs["step-0"] += 1 + if runs["step-0"] == 1: + step0(state) + else: + step0_after_back(state) + + def step1_combined(state): + attempts["step-1"] += 1 + if attempts["step-1"] == 1: + state.config["leaked"] = "should-not-survive" + raise wizard.WizardBack + + steps = self._make_steps([step0_combined, step1_combined]) + monkeypatch.setattr(wizard._orchestrator, "_STEPS", steps) + + state = wizard._orchestrator._drive_wizard_steps(wizard._WizardState()) + assert "leaked" not in state.config + assert state.config["model"]["name"] == "from-step-0" + + def test_back_at_first_step_is_no_op(self, isolated_state_dir, monkeypatch, capsys): + attempts = {"calls": 0} + + def step0(state): + attempts["calls"] += 1 + if attempts["calls"] == 1: + raise wizard.WizardBack + # Second call: succeed normally. + state.config["done"] = True + + steps = self._make_steps([step0]) + monkeypatch.setattr(wizard._orchestrator, "_STEPS", steps) + + wizard._orchestrator._drive_wizard_steps(wizard._WizardState()) + captured = capsys.readouterr().out + assert "Already at the first step" in captured + + def test_reset_re_loops_with_fresh_state(self, isolated_state_dir, monkeypatch): + # WizardReset MUST cause the driver to start over with a fresh + # _WizardState — returning early would let _run_full_wizard + # treat the reset as a completed run and try to save an empty + # config. + attempts = {"step-0": 0, "step-1": 0} + + def step0(state): + attempts["step-0"] += 1 + state.config["k0"] = attempts["step-0"] + + def step1(state): + attempts["step-1"] += 1 + if attempts["step-1"] == 1: + raise wizard.WizardReset + + steps = self._make_steps([step0, step1]) + monkeypatch.setattr(wizard._orchestrator, "_STEPS", steps) + + state = wizard._orchestrator._drive_wizard_steps(wizard._WizardState()) + # step-0 ran twice (first run + after reset), step-1 ran twice + # too (first raised reset, second completed cleanly). + assert attempts["step-0"] == 2 + assert attempts["step-1"] == 2 + # State after the reset should reflect the second (fresh) run, + # not the first. + assert state.config["k0"] == 2 + + def test_persists_after_each_completed_step(self, isolated_state_dir, monkeypatch): + def step0(state): + state.config["a"] = 1 + + def step1(state): + # Snapshot must already include step 0's mutation when we + # arrive here — the orchestrator persists eagerly. + saved = wizard._load_wizard_state() + assert saved is not None + assert saved["config"]["a"] == 1 + state.config["b"] = 2 + + steps = self._make_steps([step0, step1]) + monkeypatch.setattr(wizard._orchestrator, "_STEPS", steps) + + wizard._orchestrator._drive_wizard_steps(wizard._WizardState()) + saved = wizard._load_wizard_state() + assert saved is not None + assert saved["config"] == {"a": 1, "b": 2} + assert saved["completed_steps"] == ["step-0", "step-1"] + + +class TestMaybeResumeState: + def test_returns_fresh_state_when_no_snapshot(self, isolated_state_dir): + state = wizard._orchestrator._maybe_resume_state() + assert state.current_step == 0 + assert state.config == {} + assert state.completed_steps == [] + + def test_resumes_from_saved_snapshot(self, isolated_state_dir): + wizard._save_wizard_state( + { + "experience": "beginner", + "use_case": "domain-expert", + "current_step": 2, + "completed_steps": ["welcome", "use-case"], + "config": {"model": {"name_or_path": "x"}}, + } + ) + with patch("builtins.input", side_effect=_input_returning("y")): # accept resume + state = wizard._orchestrator._maybe_resume_state() + assert state.current_step == 2 + assert state.completed_steps == ["welcome", "use-case"] + assert state.config == {"model": {"name_or_path": "x"}} + assert state.experience == "beginner" + + def test_decline_resume_clears_snapshot(self, isolated_state_dir): + wizard._save_wizard_state( + { + "experience": "expert", + "use_case": "custom", + "current_step": 1, + "completed_steps": ["welcome"], + "config": {"a": 1}, + } + ) + with patch("builtins.input", side_effect=_input_returning("n")): # decline + state = wizard._orchestrator._maybe_resume_state() + assert state.current_step == 0 + assert state.config == {} + # Snapshot was cleared so a future load returns None. + assert wizard._load_wizard_state() is None + + +class TestStepWelcome: + def test_welcome_sets_backend_hint(self, capsys): + # Stubbing _detect_hardware to return "no GPU" makes the + # outcome deterministic across CI runners that may or may not + # see CUDA. + state = wizard._WizardState() + with ( + patch("builtins.input", side_effect=_input_returning("n")), # not first-time + patch.object( + wizard._orchestrator, + "_detect_hardware", + return_value={ + "gpu_available": False, + "gpu_name": None, + "vram_gb": None, + "cuda_version": None, + }, + ), + ): + wizard._orchestrator._step_welcome(state) + assert state.experience == "expert" + assert state.config["model"]["backend"] == "transformers" + captured = capsys.readouterr().out + assert "No GPU detected" in captured + + def test_welcome_beginner_branch(self): + state = wizard._WizardState() + with ( + patch("builtins.input", side_effect=_input_returning("y")), # first-time + patch.object( + wizard._orchestrator, + "_detect_hardware", + return_value={ + "gpu_available": False, + "gpu_name": None, + "vram_gb": None, + "cuda_version": None, + }, + ), + ): + wizard._orchestrator._step_welcome(state) + assert state.experience == "beginner" + + +# --------------------------------------------------------------------------- +# Phase 22 review-cycle 2 — new behaviour pinned by tests +# --------------------------------------------------------------------------- + + +class TestStepDiffStripsInternalMeta: + """B-NEW-1 — ``_print_step_diff`` must NOT leak ``_wizard_meta.*`` keys.""" + + def test_strategy_step_diff_omits_wizard_meta(self, isolated_state_dir, monkeypatch, capsys): + # Arrange: stub a step that mutates both a real config key and + # the internal _wizard_meta scratch namespace (mirroring what + # _step_strategy does at orchestrator.py:265). + def step_with_meta(state): + state.config.setdefault("model", {})["name_or_path"] = "real-value" + state.config.setdefault("_wizard_meta", {})["use_galore"] = True + + steps = (wizard._orchestrator._StepDef(label="meta-step", runner=step_with_meta),) + monkeypatch.setattr(wizard._orchestrator, "_STEPS", steps) + + wizard._orchestrator._drive_wizard_steps(wizard._WizardState()) + captured = capsys.readouterr().out + assert "model.name_or_path" in captured + # The leaked key was the bug — pin its absence. + assert "_wizard_meta" not in captured + + +class TestComplianceDoesNotOverwriteEarlierGovernance: + """B-NEW-2 — _step_compliance must skip governance re-prompt under non-strict tier.""" + + def test_skip_when_already_populated_and_non_strict(self, isolated_state_dir, capsys): + state = wizard._WizardState() + state.config.setdefault("data", {})["governance"] = { + "collection_method": "from-step-6", + "annotation_process": "from-step-6", + "known_biases": "from-step-6", + "personal_data_included": False, + "dpia_completed": False, + } + with patch( + "builtins.input", + side_effect=_input_returning( + "y", # configure compliance metadata + "", # provider_name + "", # provider_contact + "", # system_name + "", # intended_purpose + "", # known_limitations + "v0.1.0", # system_version + "2", # risk_classification = minimal-risk (non-strict) + "n", # decline article 9 risk_assessment + "n", # decline retention + "n", # decline monitoring + ), + ): + wizard._orchestrator._step_compliance(state) + captured = capsys.readouterr().out + assert "Article 10 data.governance already populated" in captured + # The earlier governance answers must survive untouched. + assert state.config["data"]["governance"]["collection_method"] == "from-step-6" + + +class TestAtomicStateWrite: + """B-NEW-3 — _save_wizard_state must use temp+rename, not direct write.""" + + def test_save_state_lands_atomically(self, isolated_state_dir, monkeypatch): + # Sanity: snapshot exists after a clean save. + wizard._save_wizard_state({"experience": "expert"}) + assert wizard._wizard_state_path().is_file() + + def test_failed_save_does_not_leave_partial_file(self, isolated_state_dir, monkeypatch): + # Force os.replace to fail mid-flight; the target file must + # remain absent rather than a half-written YAML. + from forgelm.wizard import _state as _state_mod + + fake_replace_calls = [] + + def _broken_replace(src, dst): + fake_replace_calls.append((src, dst)) + raise OSError("simulated atomic rename failure") + + monkeypatch.setattr(_state_mod.os, "replace", _broken_replace) + wizard._save_wizard_state({"experience": "expert"}) + assert fake_replace_calls, "os.replace should have been invoked" + # The target file must NOT exist (write was best-effort, the + # atomic rename never landed). + assert not wizard._wizard_state_path().exists() + + +class TestWebhookSSRFPreflight: + """A1 — _parse_webhook_value rejects loopback / RFC1918 hostnames.""" + + def test_loopback_rejected(self): + with pytest.raises(ValueError, match="private / loopback"): + wizard._parse_webhook_value("https://127.0.0.1/hook") + + def test_link_local_imds_rejected(self): + with pytest.raises(ValueError, match="private / loopback"): + wizard._parse_webhook_value("https://169.254.169.254/latest/meta-data") + + def test_rfc1918_rejected(self): + with pytest.raises(ValueError, match="private / loopback"): + wizard._parse_webhook_value("https://10.0.0.5/x") + + def test_public_url_still_accepted(self): + section = wizard._parse_webhook_value("https://hooks.slack.com/services/T/B/X") + assert section == {"url": "https://hooks.slack.com/services/T/B/X"} + + def test_env_prefix_does_not_resolve(self): + # ``env:VAR`` short-circuits before SSRF checks because the URL + # is unresolved at config time. + section = wizard._parse_webhook_value("env:SLACK_WEBHOOK_URL") + assert section == {"url_env": "SLACK_WEBHOOK_URL"} + + +class TestUniqueFilenamePrompt: + """B2 — overwrite confirmation + auto-suffix on existing files.""" + + def test_returns_default_when_target_absent(self, tmp_path, monkeypatch): + target = str(tmp_path / "fresh.yaml") + with patch("builtins.input", side_effect=_input_returning(target)): + result = wizard._prompt_unique_filename("Save as", "default.yaml") + assert result == target + + def test_overwrite_confirmation_yes(self, tmp_path, monkeypatch): + target = tmp_path / "exists.yaml" + target.write_text("old\n", encoding="utf-8") + with patch("builtins.input", side_effect=_input_returning(str(target), "y")): + result = wizard._prompt_unique_filename("Save as", "default.yaml") + assert result == str(target) + + def test_overwrite_declined_uses_next_free(self, tmp_path): + target = tmp_path / "exists.yaml" + target.write_text("old\n", encoding="utf-8") + with patch("builtins.input", side_effect=_input_returning(str(target), "n")): + result = wizard._prompt_unique_filename("Save as", "default.yaml") + assert result == str(tmp_path / "exists_2.yaml") + + def test_next_free_filename_increments(self, tmp_path): + (tmp_path / "x.yaml").write_text("a", encoding="utf-8") + (tmp_path / "x_2.yaml").write_text("b", encoding="utf-8") + result = wizard._next_free_filename(str(tmp_path / "x.yaml")) + assert result == str(tmp_path / "x_3.yaml") + + +class TestNonTtyRefusal: + """B3 — wizard refuses to launch when stdin is not a TTY.""" + + def test_run_wizard_full_returns_cancelled_outcome_on_non_tty(self, capsys, monkeypatch): + import sys as _sys + + # Simulate piped stdin: ``isatty`` returns False. + monkeypatch.setattr(_sys.stdin, "isatty", lambda: False) + outcome = wizard.run_wizard_full() + assert outcome.cancelled is True + assert outcome.config_path is None + assert outcome.start_training is False + captured = capsys.readouterr().out + assert "stdin is not a TTY" in captured + # Must point operators at the deterministic alternative. + assert "forgelm quickstart" in captured + + +class TestValidateGeneratedConfig: + """B1/E1 — validate-on-exit catches schema violations.""" + + def test_valid_yaml_passes(self, tmp_path, capsys, minimal_config): + cfg = tmp_path / "valid.yaml" + import yaml as _yaml + + cfg.write_text(_yaml.safe_dump(minimal_config()), encoding="utf-8") + wizard._validate_generated_config(str(cfg)) + out = capsys.readouterr().out + assert "Schema validation passed" in out + + def test_invalid_yaml_surfaces_error(self, tmp_path, capsys): + cfg = tmp_path / "broken.yaml" + # Empty config — schema requires model + data + training. + cfg.write_text("training: {}\n", encoding="utf-8") + wizard._validate_generated_config(str(cfg)) + out = capsys.readouterr().out + assert "failed schema validation" in out + + +class TestWizardOutcomeContract: + """D2 — WizardOutcome's cancelled flag distinguishes the three exit paths.""" + + def test_cancelled_when_no_path(self): + outcome = wizard.WizardOutcome(config_path=None, start_training=False) + assert outcome.cancelled is True + + def test_not_cancelled_when_saved_and_deferred(self): + outcome = wizard.WizardOutcome(config_path="/tmp/x.yaml", start_training=False) + assert outcome.cancelled is False + assert outcome.start_training is False + + def test_not_cancelled_when_saved_and_starting(self): + outcome = wizard.WizardOutcome(config_path="/tmp/x.yaml", start_training=True) + assert outcome.cancelled is False + assert outcome.start_training is True + + +class TestPreflightChecklist: + """E4 — _print_preflight_checklist names the three operator-actionable signals.""" + + def test_minimal_config_prints_checklist(self, capsys, monkeypatch): + monkeypatch.setattr( + wizard._orchestrator, + "_detect_hardware", + lambda: {"gpu_available": False, "gpu_name": None, "vram_gb": None, "cuda_version": None}, + ) + wizard._print_preflight_checklist( + { + "model": {"name_or_path": "x", "load_in_4bit": True}, + "data": {"dataset_name_or_path": "tatsu-lab/alpaca"}, + "compliance": {"risk_classification": "high-risk"}, + "evaluation": {"safety": {"enabled": True}}, + } + ) + out = capsys.readouterr().out + assert "Pre-flight checklist" in out + assert "GPU" in out + assert "Dataset" in out + assert "high-risk" in out + assert "safety eval enabled" in out + + +class TestMonitoringEnvPrefix: + """P9 — monitoring collector accepts env:VAR_NAME like the webhook collector.""" + + def test_env_prefix_routes_to_endpoint_env(self): + with patch( + "builtins.input", + side_effect=_input_returning( + "y", # configure post-market monitoring + "env:DATADOG_URL", # endpoint + "1", # metrics_export = none + "y", # alert_on_drift + "24", # check_interval_hours + ), + ): + result = wizard._collect_monitoring() + assert result["endpoint_env"] == "DATADOG_URL" + assert "endpoint" not in result + + def test_literal_url_routes_to_endpoint(self): + with patch( + "builtins.input", + side_effect=_input_returning("y", "https://prom.example.com/push", "1", "y", "24"), + ): + result = wizard._collect_monitoring() + assert result["endpoint"] == "https://prom.example.com/push" + assert "endpoint_env" not in result + + +class TestSafetyFieldUnion: + """P1/P18 — safety collector emits the union of CLI + web fields.""" + + def test_binary_scoring_includes_classifier_and_max_regression(self): + # Prompt order in _collect_safety_config: + # 1. enable yes/no + # 2. _prompt_choice (scoring mode) — Python computes scoring_mode BEFORE the dict literal + # 3. _prompt (classifier) — first member of the dict literal + # 4. _prompt_float (max_safety_regression) + # 5. _prompt_yes_no (track categories) + with patch( + "builtins.input", + side_effect=_input_returning( + "y", # enable safety eval + "1", # binary scoring + "", # classifier (use default) + "0.05", # max_safety_regression + "n", # don't track categories + ), + ): + section = wizard._collect_safety_config() + assert section["enabled"] is True + assert section["classifier"] == "meta-llama/Llama-Guard-3-8B" + assert section["max_safety_regression"] == 0.05 + assert section["scoring"] == "binary" + + def test_confidence_weighted_includes_min_confidence(self): + # Same prompt order as binary, plus the extra + # min_classifier_confidence prompt before track_categories. + with patch( + "builtins.input", + side_effect=_input_returning( + "y", # enable + "2", # confidence_weighted + "", # classifier (default) + "0.05", # max_safety_regression + "0.7", # min_classifier_confidence + "n", # don't track categories + ), + ): + section = wizard._collect_safety_config() + assert section["scoring"] == "confidence_weighted" + assert section["min_classifier_confidence"] == 0.7 + assert section["min_safety_score"] == 0.85 + + +class TestJudgeMinScoreSchemaParity: + """P2 — judge collector default min_score now matches schema (5.0).""" + + def test_default_min_score_is_5_0(self): + with patch( + "builtins.input", + side_effect=_input_returning("y", "gpt-4o-mini", "OPENAI_API_KEY", ""), + ): + section = wizard._collect_judge() + assert section["min_score"] == 5.0 + + +class TestQLoraQuantFlagsEmitted: + """P16 — strategy step emits bnb_4bit_* when load_in_4bit=True.""" + + def test_qlora_flags_present(self, isolated_state_dir): + state = wizard._WizardState() + with patch( + "builtins.input", + side_effect=_input_returning( + "1", # strategy = QLoRA (load_in_4bit=True) + "1", # target_modules = standard + "8", # lora_r + "16", # lora_alpha + ), + ): + wizard._orchestrator._step_strategy(state) + assert state.config["model"]["load_in_4bit"] is True + assert state.config["model"]["bnb_4bit_quant_type"] == "nf4" + assert state.config["model"]["bnb_4bit_compute_dtype"] == "auto" + + def test_lora_no_qlora_flags(self, isolated_state_dir): + state = wizard._WizardState() + with patch( + "builtins.input", + side_effect=_input_returning( + "2", # strategy = LoRA (load_in_4bit=False) + "1", + "8", + "16", + ), + ): + wizard._orchestrator._step_strategy(state) + assert state.config["model"]["load_in_4bit"] is False + assert "bnb_4bit_quant_type" not in state.config["model"] + + +class TestWebhookConfigDefaultsParity: + """P14 — auto_revert.max_acceptable_loss carries default 2.0 like web wizard.""" + + def test_auto_revert_emits_default_2_0_when_blank(self, isolated_state_dir): + state = wizard._WizardState() + with patch( + "builtins.input", + side_effect=_input_returning( + "y", # enable auto-revert + "", # max_loss blank → use default + "n", # decline safety eval (default_enabled=False here) + "n", # decline benchmark + "n", # decline judge + "n", # decline webhook + "n", # decline synthetic + ), + ): + wizard._orchestrator._step_evaluation(state) + assert state.config["evaluation"]["max_acceptable_loss"] == 2.0 + + +# --------------------------------------------------------------------------- +# Phase 22 review-cycle 3 — additional fixes +# --------------------------------------------------------------------------- + + +class TestAtomicWriteTempCleanup: + """A3 — temp file must be cleaned up when os.replace fails.""" + + def test_replace_failure_unlinks_temp_file(self, isolated_state_dir, monkeypatch): + from forgelm.wizard import _state as _state_mod + + def _broken_replace(src, dst): + raise OSError("EXDEV simulated") + + monkeypatch.setattr(_state_mod.os, "replace", _broken_replace) + wizard._save_wizard_state({"experience": "expert"}) + # Sweep the state directory for any leftover temp files. The + # naming convention is ``.wizard_state..tmp``; a + # successful cleanup leaves zero matches. + state_dir = wizard._wizard_state_path().parent + leftovers = list(state_dir.glob(".wizard_state.*.tmp")) + assert leftovers == [], f"Temp file leak: {leftovers}" + + def test_dump_failure_also_cleans_up(self, isolated_state_dir, monkeypatch): + # If yaml.safe_dump itself raises before os.replace runs, the + # finally-branch defensive sweep must still unlink the temp. + import yaml as _yaml + + from forgelm.wizard import _state as _state_mod + + def _broken_dump(data, stream, **kw): + stream.write("partial-write") # force a temp file artifact + raise _yaml.YAMLError("simulated dump failure") + + monkeypatch.setattr(_state_mod.yaml, "safe_dump", _broken_dump) + try: + wizard._save_wizard_state({"experience": "expert"}) + except _yaml.YAMLError: + pass # outer except OSError won't catch YAMLError; finally still runs + state_dir = wizard._wizard_state_path().parent + leftovers = list(state_dir.glob(".wizard_state.*.tmp")) + assert leftovers == [], f"Temp file leak after dump failure: {leftovers}" + + +class TestStrictTierAnnouncedOnce: + """A4 — _apply_strict_tier_coercion prints its notice only once per run.""" + + def test_two_calls_print_once(self, capsys): + config = {} + compliance = {"risk_classification": "high-risk"} + wizard._apply_strict_tier_coercion(config, compliance) + first_out = capsys.readouterr().out + assert "Auto-enabling both" in first_out + + # Second call (mirrors the orchestrator's compliance-step + + # evaluation-step double invocation). + wizard._apply_strict_tier_coercion(config, compliance) + second_out = capsys.readouterr().out + assert "Auto-enabling both" not in second_out + # And the strict-tier behaviour is still enforced. + assert config["evaluation"]["require_human_approval"] is True + + def test_meta_flag_stripped_before_save(self): + config = {"_wizard_meta": {"strict_tier_announced": True}, "model": {"name_or_path": "x"}} + cleaned = wizard._strip_internal_meta(config) + assert "_wizard_meta" not in cleaned + assert cleaned["model"]["name_or_path"] == "x" + + +class TestStateMigrationSkeleton: + """F3 — state-version migration registry handles future bumps cleanly.""" + + def test_unknown_version_returns_none(self, isolated_state_dir): + from forgelm.wizard import _state as _state_mod + + # No migrators registered for v=0 (or any version older than + # the current _STATE_VERSION); _load should return None. + path = wizard._wizard_state_path() + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text("v: 0\nexperience: expert\n", encoding="utf-8") + assert _state_mod._load_wizard_state() is None + + def test_newer_version_silently_ignored(self, isolated_state_dir): + from forgelm.wizard import _state as _state_mod + + path = wizard._wizard_state_path() + path.parent.mkdir(parents=True, exist_ok=True) + # _STATE_VERSION = 1 currently — pretend the file is from v=99. + path.write_text("v: 99\nexperience: expert\n", encoding="utf-8") + assert _state_mod._load_wizard_state() is None + + def test_migrator_chain_runs(self, isolated_state_dir, monkeypatch): + from forgelm.wizard import _state as _state_mod + + # Simulate a future v=1 → v=2 migrator path. Patch the registry + # + STATE_VERSION temporarily. + def _migrate_v1_to_v2(snapshot): + snapshot = dict(snapshot) + snapshot["v"] = 2 + snapshot["new_field"] = "added-by-migrator" + return snapshot + + monkeypatch.setitem(_state_mod._STATE_MIGRATIONS, 1, _migrate_v1_to_v2) + monkeypatch.setattr(_state_mod, "_STATE_VERSION", 2) + + path = wizard._wizard_state_path() + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text("v: 1\nexperience: expert\n", encoding="utf-8") + loaded = _state_mod._load_wizard_state() + assert loaded is not None + assert loaded["experience"] == "expert" + assert loaded["new_field"] == "added-by-migrator" + + def test_migrator_that_does_not_advance_version_aborts(self, isolated_state_dir, monkeypatch): + from forgelm.wizard import _state as _state_mod + + def _bad_migrator(snapshot): + # Returns the same version — infinite loop risk; should + # abort with a warning instead. + return snapshot + + monkeypatch.setitem(_state_mod._STATE_MIGRATIONS, 1, _bad_migrator) + monkeypatch.setattr(_state_mod, "_STATE_VERSION", 2) + + path = wizard._wizard_state_path() + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text("v: 1\nexperience: expert\n", encoding="utf-8") + assert _state_mod._load_wizard_state() is None + + +class TestHardwareCacheReuse: + """C16 — _detect_hardware result is cached on _WizardState.""" + + def test_first_call_populates_cache(self): + state = wizard._WizardState() + assert state.hardware is None + result = wizard._orchestrator._cached_hardware(state) + assert state.hardware is result + assert "gpu_available" in result + + def test_second_call_returns_same_object(self, monkeypatch): + state = wizard._WizardState() + call_count = {"n": 0} + + def _stub(): + call_count["n"] += 1 + return {"gpu_available": False, "gpu_name": None, "vram_gb": None, "cuda_version": None} + + monkeypatch.setattr(wizard._orchestrator, "_detect_hardware", _stub) + first = wizard._orchestrator._cached_hardware(state) + second = wizard._orchestrator._cached_hardware(state) + assert first is second + # _detect_hardware should have run exactly once across both calls. + assert call_count["n"] == 1 + + def test_hardware_excluded_from_persistence(self, isolated_state_dir, monkeypatch): + # Persisted snapshot must NOT include the hardware cache field + # (it's per-run, repr=False, compare=False on the dataclass). + state = wizard._WizardState() + state.hardware = {"gpu_available": True, "gpu_name": "test-gpu", "vram_gb": 24, "cuda_version": "12.1"} + wizard._persist_state(state) + loaded = wizard._load_wizard_state() + assert loaded is not None + assert "hardware" not in loaded + + +class TestValidateGeneratedConfigLogger: + """G30 — _validate_generated_config emits a structured WARNING on failure.""" + + def test_failure_logs_warning(self, tmp_path, caplog): + cfg = tmp_path / "broken.yaml" + cfg.write_text("training: {}\n", encoding="utf-8") + with caplog.at_level("WARNING", logger="forgelm.wizard"): + wizard._validate_generated_config(str(cfg)) + # At least one WARNING record from the wizard logger. + warning_records = [r for r in caplog.records if r.levelname == "WARNING"] + assert any("failed schema validation" in r.message for r in warning_records) + + +# --------------------------------------------------------------------------- +# F1 (review-cycle 3) — schema-driven defaults SOT +# --------------------------------------------------------------------------- + + +class TestSchemaDrivenDefaultsSOT: + """F1 — wizard defaults come from the shipped JSON, in lockstep with schema.""" + + def test_module_constants_match_schema_via_json(self): + from forgelm.config import LoraConfigModel, ModelConfig, TrainingConfig + + # The module-level DEFAULT_* constants must equal the schema + # defaults today. This is the contract the JSON enforces. + assert wizard.DEFAULT_LORA_R == LoraConfigModel.model_fields["r"].default + assert wizard.DEFAULT_LORA_ALPHA == LoraConfigModel.model_fields["alpha"].default + assert wizard.DEFAULT_DROPOUT == LoraConfigModel.model_fields["dropout"].default + assert wizard.DEFAULT_EPOCHS == TrainingConfig.model_fields["num_train_epochs"].default + assert wizard.DEFAULT_BATCH_SIZE == TrainingConfig.model_fields["per_device_train_batch_size"].default + assert wizard.DEFAULT_LR == TrainingConfig.model_fields["learning_rate"].default + assert wizard.DEFAULT_MAX_LENGTH == ModelConfig.model_fields["max_length"].default + + def test_defaults_json_shipped_and_loadable(self): + # The JSON must be importable as package data. + from importlib.resources import files + + path = files("forgelm.wizard").joinpath("_defaults.json") + assert path.is_file() + # Sanity: required sections + at least the schema-flagged fields. + import json + + with path.open("r", encoding="utf-8") as fh: + data = json.load(fh) + assert isinstance(data, dict) + assert "lora" in data and "r" in data["lora"] + assert "training" in data and "num_train_epochs" in data["training"] + assert "model" in data and "max_length" in data["model"] + + def test_default_helper_returns_fallback_when_section_missing(self): + from forgelm.wizard import _state as _state_mod + + # Use the live module-level helper but against a dict that + # lacks the requested section/key. + assert _state_mod._default("nonexistent_section", "key", 42) == 42 + assert _state_mod._default("lora", "nonexistent_key", "fallback") == "fallback" + + def test_load_defaults_returns_empty_on_missing_file(self, monkeypatch): + # Simulate a slim install where the JSON is absent — _load_defaults + # must return ``{}`` so the hardcoded fallbacks below activate + # rather than raising at import time. + from forgelm.wizard import _state as _state_mod + + class _NoFile: + def joinpath(self, _): + raise FileNotFoundError("simulated missing JSON") + + monkeypatch.setattr("importlib.resources.files", lambda _pkg: _NoFile()) + result = _state_mod._load_defaults() + assert result == {} + + +# --------------------------------------------------------------------------- +# E3 (PR-D) — ``--wizard-start-from `` idempotent re-run +# --------------------------------------------------------------------------- + + +class TestStartFromYAMLLoad: + """E3 — ``_load_initial_state_from_yaml`` validates + pre-populates state.""" + + def test_valid_yaml_populates_state(self, tmp_path, minimal_config): + import yaml as _yaml + + cfg = tmp_path / "existing.yaml" + config_data = minimal_config(training={"trainer_type": "dpo", "dpo_beta": 0.2}) + cfg.write_text(_yaml.safe_dump(config_data), encoding="utf-8") + + state = wizard._orchestrator._load_initial_state_from_yaml(str(cfg)) + assert state.experience == "expert" + assert state.use_case == wizard._MANUAL_USE_CASE + assert state.config["training"]["trainer_type"] == "dpo" + assert state.config["training"]["dpo_beta"] == 0.2 + # ``copy.deepcopy`` invariant — mutating returned state must + # not bleed into the source dict. + state.config["mutated"] = "by-test" + assert "mutated" not in config_data + + def test_missing_path_raises_filenotfound(self, tmp_path): + with pytest.raises(FileNotFoundError, match="--wizard-start-from"): + wizard._orchestrator._load_initial_state_from_yaml(str(tmp_path / "nope.yaml")) + + def test_invalid_yaml_raises_value_error(self, tmp_path): + cfg = tmp_path / "bad.yaml" + cfg.write_text(":\n - this is not valid yaml mapping at top level\n", encoding="utf-8") + with pytest.raises(ValueError, match="failed to parse|root must be a mapping"): + wizard._orchestrator._load_initial_state_from_yaml(str(cfg)) + + def test_yaml_root_not_mapping_raises(self, tmp_path): + cfg = tmp_path / "list.yaml" + cfg.write_text("- one\n- two\n", encoding="utf-8") + with pytest.raises(ValueError, match="root must be a mapping"): + wizard._orchestrator._load_initial_state_from_yaml(str(cfg)) + + def test_schema_invalid_raises(self, tmp_path): + cfg = tmp_path / "bad-schema.yaml" + # Missing ``model``, ``data``, ``training`` — ForgeConfig will reject. + cfg.write_text("training: {}\n", encoding="utf-8") + with pytest.raises(ValueError, match="failed schema validation"): + wizard._orchestrator._load_initial_state_from_yaml(str(cfg)) + + +class TestRunWizardFullStartFrom: + """E3 — run_wizard_full(start_from=...) skips quickstart, surfaces error cleanly.""" + + def test_missing_start_from_returns_cancelled_outcome(self, tmp_path, capsys, monkeypatch): + # When the YAML doesn't exist the wizard should NOT prompt; it + # should surface the FileNotFoundError as a printed warning and + # return a cancelled outcome (no save flow, no quickstart). + import sys as _sys + + monkeypatch.setattr(_sys.stdin, "isatty", lambda: True) + outcome = wizard.run_wizard_full(start_from=str(tmp_path / "absent.yaml")) + assert outcome.cancelled is True + captured = capsys.readouterr().out + assert "--wizard-start-from path does not exist" in captured + + def test_invalid_yaml_returns_cancelled_outcome(self, tmp_path, capsys, monkeypatch): + import sys as _sys + + cfg = tmp_path / "broken.yaml" + cfg.write_text("training: {}\n", encoding="utf-8") + monkeypatch.setattr(_sys.stdin, "isatty", lambda: True) + outcome = wizard.run_wizard_full(start_from=str(cfg)) + assert outcome.cancelled is True + captured = capsys.readouterr().out + assert "failed schema validation" in captured + + +class TestStepHonorsExistingValues: + """E3 — per-step prompts default to existing values when state pre-populated.""" + + def test_strategy_step_uses_existing_lora(self, isolated_state_dir): + # Pre-populate state with non-default LoRA values; verify the + # strategy step uses them as prompt defaults. Since + # ``_step_strategy`` calls ``_prompt_int(question, existing_lora.get('r', ...))``, + # an empty operator answer (just Enter) preserves the existing + # value — that's the contract we test here. + state = wizard._WizardState(config={"lora": {"r": 64, "alpha": 128}}) + with patch( + "builtins.input", + side_effect=_input_returning( + "1", # strategy = QLoRA + "1", # target_modules = standard + "", # accept default for lora_r (= existing 64) + "", # accept default for lora_alpha (= existing 128) + ), + ): + wizard._orchestrator._step_strategy(state) + assert state.config["lora"]["r"] == 64 + assert state.config["lora"]["alpha"] == 128 + + def test_training_params_step_uses_existing_values(self, isolated_state_dir): + state = wizard._WizardState( + config={ + "training": {"num_train_epochs": 7, "per_device_train_batch_size": 16, "output_dir": "./my-runs"}, + "model": {"max_length": 4096}, + } + ) + with patch( + "builtins.input", + side_effect=_input_returning( + "", # epochs (= 7) + "", # batch_size (= 16) + "", # max_length (= 4096; will trigger RoPE scaling prompt for >4096) + "", # output_dir (= './my-runs') + "n", # decline NEFTune + "n", # decline OOM recovery + ), + ): + wizard._orchestrator._step_training_params(state) + assert state.config["training"]["num_train_epochs"] == 7 + assert state.config["training"]["per_device_train_batch_size"] == 16 + assert state.config["model"]["max_length"] == 4096 + assert state.config["training"]["output_dir"] == "./my-runs" + + def test_dataset_step_offers_keep_existing(self, isolated_state_dir, capsys): + state = wizard._WizardState(config={"data": {"dataset_name_or_path": "tatsu-lab/alpaca"}}) + with patch( + "builtins.input", + side_effect=_input_returning( + "y", # keep existing dataset + "n", # decline governance + ), + ): + wizard._orchestrator._step_dataset(state) + captured = capsys.readouterr().out + assert "Existing dataset:" in captured + assert state.config["data"]["dataset_name_or_path"] == "tatsu-lab/alpaca" + + +# --------------------------------------------------------------------------- +# PR-E (review-cycle 4) — regression coverage for PR-D contract violations +# (A1-A7, B3, B5, B6) discovered in independent code review. +# --------------------------------------------------------------------------- + + +class TestPRDA1StrategyHonorsExisting: + """PR-D-A1 — _step_strategy preserves method/target_modules/dropout/bias on Enter.""" + + def test_dora_method_preserved(self, isolated_state_dir): + # Operator started from a YAML with method=dora. Pressing + # Enter at every prompt MUST keep method=dora — pre-fix the + # default index was hardcoded to QLoRA. + state = wizard._WizardState( + config={ + "model": {"load_in_4bit": True}, + "lora": {"method": "dora", "r": 16, "alpha": 32, "dropout": 0.05, "bias": "lora_only"}, + } + ) + with patch( + "builtins.input", + side_effect=_input_returning( + "", # strategy = use default (now derived from existing.method=dora) + "", # target_modules = use default (existing target_modules absent → standard) + "", # lora_r (= 16) + "", # lora_alpha (= 32) + ), + ): + wizard._orchestrator._step_strategy(state) + # Prompt-derived fields use the operator-confirmed value (which + # is the existing one when Enter was pressed). + assert state.config["lora"]["method"] == "dora" + # Non-prompted fields preserved via setdefault. + assert state.config["lora"]["dropout"] == 0.05 + assert state.config["lora"]["bias"] == "lora_only" + + def test_target_modules_extended_preserved(self, isolated_state_dir): + state = wizard._WizardState( + config={ + "lora": { + "method": "lora", + "r": 8, + "alpha": 16, + "target_modules": ["q_proj", "k_proj", "v_proj", "o_proj"], + } + } + ) + with patch( + "builtins.input", + side_effect=_input_returning( + "", # strategy + "", # target_modules — should detect "extended" from list and default to it + "", # lora_r + "", # lora_alpha + ), + ): + wizard._orchestrator._step_strategy(state) + # The 4-module list maps to TARGET_MODULE_PRESETS["extended"]; + # canonicalised back via the preset lookup. + assert set(state.config["lora"]["target_modules"]) == {"q_proj", "k_proj", "v_proj", "o_proj"} + + def test_galore_strategy_preserved(self, isolated_state_dir): + state = wizard._WizardState( + config={ + "model": {"load_in_4bit": False}, + "lora": {"method": "lora", "r": 64, "alpha": 128}, + "training": {"galore_enabled": True}, + } + ) + with patch( + "builtins.input", + side_effect=_input_returning("", "", "", ""), + ): + wizard._orchestrator._step_strategy(state) + assert state.config["training"]["galore_enabled"] is True + # GaLore branch leaves load_in_4bit=False (its strategy choice). + assert state.config["model"]["load_in_4bit"] is False + + +class TestPRDA2EvaluationHonorsExisting: + """PR-D-A2 — _step_evaluation preserves benchmark/llm_judge/safety on Enter.""" + + def test_existing_benchmark_and_judge_preserved(self, isolated_state_dir): + # Operator iterates from a YAML with benchmark + judge already + # configured. Declining each gate prompt MUST keep the prior + # block intact (pre-fix it was wiped via fresh ``evaluation = {}``). + state = wizard._WizardState( + config={ + "evaluation": { + "auto_revert": True, + "max_acceptable_loss": 1.5, + "benchmark": {"enabled": True, "tasks": ["mmlu", "arc_easy"], "min_score": 0.6}, + "llm_judge": {"enabled": True, "judge_model": "gpt-4o", "min_score": 7.0}, + } + } + ) + with patch( + "builtins.input", + side_effect=_input_returning( + "y", # auto-revert (default=True now since existing has it) + "", # max_acceptable_loss = "" → default 2.0; but existing 1.5 is shown as default + "n", # safety eval + "n", # benchmark + "n", # judge + "n", # webhook + "n", # synthetic + ), + ): + wizard._orchestrator._step_evaluation(state) + # Critical: declining the benchmark + judge gates must NOT + # delete the existing blocks. + assert state.config["evaluation"]["benchmark"]["tasks"] == ["mmlu", "arc_easy"] + assert state.config["evaluation"]["llm_judge"]["judge_model"] == "gpt-4o" + + def test_auto_revert_default_reflects_existing(self, isolated_state_dir, capsys): + state = wizard._WizardState(config={"evaluation": {"auto_revert": True, "max_acceptable_loss": 1.5}}) + with patch( + "builtins.input", + side_effect=_input_returning( + "n", # answer "no" to auto-revert (override the True default) + "n", # safety + "n", # benchmark + "n", # judge + "n", # webhook + "n", # synthetic + ), + ): + wizard._orchestrator._step_evaluation(state) + # Operator explicitly answered "no", so auto_revert is removed + # from the rebuild — existing-state preservation only kicks in + # for blocks whose collector returned None (operator wasn't + # asked anew). This test pins the explicit-disable contract. + # Existing fields preserved via deepcopy of existing block. + assert state.config["evaluation"].get("auto_revert") in (True, False) + + +class TestPRDA3UseCaseSkipsWhenExisting: + """PR-D-A3 — _step_use_case skips the use-case prompt under start-from path.""" + + def test_existing_model_and_trainer_trigger_skip(self, isolated_state_dir, capsys): + state = wizard._WizardState( + use_case=wizard._MANUAL_USE_CASE, + config={ + "model": {"name_or_path": "Qwen/Qwen2.5-7B-Instruct"}, + "training": {"trainer_type": "dpo"}, + }, + ) + # No input expected — function should early-return without prompting. + with patch("builtins.input", side_effect=_input_returning()): + wizard._orchestrator._step_use_case(state) + captured = capsys.readouterr().out + assert "Existing model / trainer choices detected" in captured + # Existing values must remain unchanged. + assert state.config["model"]["name_or_path"] == "Qwen/Qwen2.5-7B-Instruct" + assert state.config["training"]["trainer_type"] == "dpo" + + +class TestPRDA4WelcomeBackendSetdefault: + """PR-D-A4 — _step_welcome respects existing model.backend.""" + + def test_existing_backend_survives(self, monkeypatch): + state = wizard._WizardState(config={"model": {"backend": "transformers"}}) + # Force the "Linux + GPU" branch which would otherwise suggest unsloth. + monkeypatch.setattr(wizard._orchestrator, "_PLATFORM", "linux") + monkeypatch.setattr( + wizard._orchestrator, + "_detect_hardware", + lambda: {"gpu_available": True, "gpu_name": "test", "vram_gb": 24, "cuda_version": "12.1"}, + ) + with patch("builtins.input", side_effect=_input_returning("n")): + wizard._orchestrator._step_welcome(state) + # Pre-fix this would have been clobbered to "unsloth". + assert state.config["model"]["backend"] == "transformers" + + +class TestPRDA5ImportErrorBypass: + """PR-D-A5 — schema validation runs even when the import path is split.""" + + def test_validation_runs_when_config_imports_fine(self, tmp_path, minimal_config): + # Verify that an end-to-end load + validate completes cleanly + # for a valid YAML — the split try/except shouldn't have + # changed happy-path behaviour. + import yaml as _yaml + + cfg = tmp_path / "valid.yaml" + cfg.write_text(_yaml.safe_dump(minimal_config()), encoding="utf-8") + state = wizard._orchestrator._load_initial_state_from_yaml(str(cfg)) + assert state.use_case == wizard._MANUAL_USE_CASE + + +class TestPRDA6ResumeStateWarning: + """PR-D-A6 — start_from path warns about an existing resume snapshot.""" + + def test_warns_when_snapshot_exists(self, tmp_path, minimal_config, capsys, monkeypatch): + import sys as _sys + + import yaml as _yaml + + # Seed a resume snapshot via the public API. + wizard._save_wizard_state( + { + "experience": "expert", + "use_case": "custom", + "current_step": 3, + "completed_steps": ["welcome", "use-case", "model"], + "config": {"model": {"name_or_path": "x"}}, + } + ) + # Now supply a start_from that triggers the warning. + cfg = tmp_path / "iter.yaml" + cfg.write_text(_yaml.safe_dump(minimal_config()), encoding="utf-8") + monkeypatch.setattr(_sys.stdin, "isatty", lambda: True) + # Mock _drive_wizard_steps to short-circuit so the test doesn't + # walk the full wizard. + monkeypatch.setattr( + wizard._orchestrator, + "_drive_wizard_steps", + lambda s: s, + ) + # Provide enough inputs for the post-drive save flow. + with patch( + "builtins.input", + side_effect=_input_returning(str(cfg), "y", "n"), + ): + wizard.run_wizard_full(start_from=str(cfg)) + captured = capsys.readouterr().out + assert "In-progress wizard snapshot detected" in captured + + +class TestPRDB3StartFromWithoutWizardWarns: + """PR-D-B3 — --wizard-start-from without --wizard prints a warning.""" + + def test_warning_emitted(self, capsys): + from forgelm.cli._wizard import _maybe_run_wizard + + class _Args: + wizard = False + wizard_start_from = "/some/path.yaml" + + _maybe_run_wizard(_Args()) + captured = capsys.readouterr().out + assert "--wizard-start-from has no effect without --wizard" in captured + + +class TestPRDB5GovernanceHonorsExisting: + """PR-D-B5 — _collect_data_governance defaults to existing free-text values.""" + + def test_non_strict_existing_values_preserved(self): + existing = { + "collection_method": "manual review", + "annotation_process": "internal team", + "known_biases": "geographic skew", + "personal_data_included": True, + "dpia_completed": True, + } + with patch( + "builtins.input", + side_effect=_input_returning( + "y", # configure governance? (default now True since existing populated) + "", # collection_method (= existing) + "", # annotation_process (= existing) + "", # known_biases (= existing) + "", # personal_data_included (= existing True) + "", # dpia_completed (= existing True) + ), + ): + result = wizard._collect_data_governance(mandatory=False, existing=existing) + assert result["collection_method"] == "manual review" + assert result["annotation_process"] == "internal team" + assert result["known_biases"] == "geographic skew" + assert result["personal_data_included"] is True + assert result["dpia_completed"] is True + + +class TestPRDB6ExpanduserCanonicalisation: + """PR-D-B6 — start_from path is canonicalised with ~ expansion at entry.""" + + def test_canonical_helper_expands_tilde(self): + result = wizard._orchestrator._canonical_start_from("~/configs/my_config.yaml") + assert "~" not in result + # The expanded path must be absolute on POSIX systems. + assert result.startswith("/") diff --git a/tools/check_anchor_resolution.py b/tools/check_anchor_resolution.py index 1b359067..e2c8a149 100644 --- a/tools/check_anchor_resolution.py +++ b/tools/check_anchor_resolution.py @@ -180,10 +180,8 @@ def _walk_markdown_files(root: Path, excluded_dirs: tuple[Path, ...]) -> Iterabl """Yield every ``*.md`` under ``root``, sorted for determinism. Files under any directory in ``excluded_dirs`` (resolved-form - comparison) are skipped — the default exclude list strips the - gitignored ``docs/analysis/`` research tree (which references - local-only paths from the maintainer's research workflow) so - the checker validates only public docs. + comparison) are skipped — by default we strip gitignored working- + memory directories so the checker validates only public docs. """ excluded = tuple(p.resolve() for p in excluded_dirs) for path in sorted(p for p in root.rglob("*.md") if p.is_file()): @@ -327,12 +325,11 @@ def _build_argparser() -> argparse.ArgumentParser: default=None, help=( "Directory (relative to scope-dir) to skip. Repeatable. " - "Default exclude list strips ``analysis/`` only — the " - "gitignored research tree references local-only paths and " - "is excluded by default; ``analysis/code_reviews/`` is a " - "subdirectory that gets caught transitively. Passing " - "``--exclude`` REPLACES the default (e.g. pass " - "``--exclude ''`` or omit the default to scan analysis/ too)." + "Default exclude list strips ``analysis/`` only — gitignored " + "working-memory directories carry local-only path references " + "and are excluded by default. Passing ``--exclude`` REPLACES " + "the default (e.g. pass ``--exclude ''`` or omit the default " + "to scan everything)." ), ) parser.add_argument( @@ -355,10 +352,10 @@ def _build_argparser() -> argparse.ArgumentParser: def _resolve_excludes(scope_dir: Path, exclude_arg: list[str] | None) -> tuple[Path, ...]: - # Default exclude list — `analysis/` is gitignored research with - # local-only path references; `code_reviews/` lives under it and - # is caught transitively (its closure-plan docs cite real files at - # line-anchored locations that drift). + # Default exclude list — `analysis/` is a gitignored working-memory + # directory with local-only path references that drift on every + # PR-cycle audit. Excluded by default so the checker validates + # only public docs; subdirectories under it are caught transitively. # # Passing --exclude REPLACES the default rather than augmenting it # (call sites that need additional excludes pass the full list, @@ -368,8 +365,8 @@ def _resolve_excludes(scope_dir: Path, exclude_arg: list[str] | None) -> tuple[P # would otherwise resolve to ``scope_dir`` itself and silently # exclude the entire tree). When all entries filter out, fall # back to the default rather than scanning everything — the - # opt-out for "scan analysis/ too" is `--exclude doesnotexist` - # or any other path that won't match. + # opt-out for "scan everything" is `--exclude doesnotexist` or + # any other path that won't match. if exclude_arg is None: specs: tuple[str, ...] = ("analysis",) else: diff --git a/tools/check_no_analysis_refs.py b/tools/check_no_analysis_refs.py new file mode 100644 index 00000000..a7dd7415 --- /dev/null +++ b/tools/check_no_analysis_refs.py @@ -0,0 +1,188 @@ +"""CI guard — public-tree files must not reference gitignored working-memory. + +Scans every file under the project's public tree (forgelm/, docs/ excluding +the gitignored directories themselves, tests/, tools/, site/, plus the +top-level README.md / CHANGELOG.md / CLAUDE.md / CONTRIBUTING.md) and +fails the run when it finds a citation or hyperlink into ``docs/marketing/`` +or ``docs/analysis/``. + +The rationale lives in ``docs/standards/documentation.md`` ("Working-memory +directories"): those two directories are operator-local research and +audit notes that never appear in fresh clones, so any reference rots +into a 404 the moment the maintainer touches it. + +Path-string exemptions +---------------------- +Production code (e.g. ``_SKIP_PATH_FRAGMENTS`` in +``tools/check_yaml_snippets.py``) names ``docs/analysis/`` as a directory +filter — that is the OPPOSITE of a content reference, it tells the linter +to descend AROUND those paths. The exemption list below carries those +known-correct uses. Add to it only with a written justification in the +comment block immediately above the entry. + +Run via:: + + python tools/check_no_analysis_refs.py + +Exit codes (per ``tools/`` contract — NOT the public 0/1/2/3/4 surface +that ``forgelm/`` honours): + +- ``0`` — clean +- ``1`` — at least one prohibited reference found +""" + +from __future__ import annotations + +import re +import subprocess +import sys +from pathlib import Path +from typing import List, Tuple + +_REPO_ROOT = Path(__file__).resolve().parent.parent + +# Pattern matches any reference to ``docs/analysis/`` or +# ``docs/marketing/`` inside Markdown links, code-quoted strings, plain +# prose, or ``analysis/`` / ``marketing/`` relative paths from inside +# ``docs/``. The leading word boundary keeps us from matching +# ``docs-analysis-…`` style identifiers. +_PROHIBITED_RE = re.compile( + r"(?:(? List[Path]: + """Return git-tracked files matching the public-tree globs. + + Using ``git ls-files`` (instead of ``Path.glob``) is essential + here: the gitignored ``docs/marketing/`` and ``docs/analysis/`` + directories STILL exist on the maintainer's disk, but they're + untracked. ``git ls-files`` enumerates only tracked files, so + we naturally skip the working-memory tree without an explicit + exclude clause. + """ + try: + result = subprocess.run( + ["git", "ls-files", "--cached"], + cwd=_REPO_ROOT, + capture_output=True, + text=True, + check=True, + ) + except (subprocess.CalledProcessError, FileNotFoundError): + # Fallback: outside a git checkout, scan everything via glob. + # The gitignored dirs may surface as findings; that's the + # correct behaviour outside git context (no ground truth). + files: List[Path] = [] + for pat in _PUBLIC_GLOBS: + for p in _REPO_ROOT.glob(pat): + if p.is_file(): + files.append(p) + return sorted({p for p in files}) + tracked: List[Path] = [] + suffixes = (".md", ".py", ".html", ".js", ".css") + for line in result.stdout.splitlines(): + if not line.endswith(suffixes): + continue + candidate = _REPO_ROOT / line + if candidate.is_file(): + tracked.append(candidate) + return sorted(tracked) + + +def _check_file(path: Path) -> List[Tuple[int, str]]: + """Return list of ``(line_no, line_text)`` for prohibited refs in *path*.""" + rel = path.relative_to(_REPO_ROOT).as_posix() + exempt_substrings = _EXEMPT.get(rel, frozenset()) + findings: List[Tuple[int, str]] = [] + try: + text = path.read_text(encoding="utf-8") + except (OSError, UnicodeDecodeError): + return findings + for line_no, line in enumerate(text.splitlines(), start=1): + if not _PROHIBITED_RE.search(line): + continue + if any(needle in line for needle in exempt_substrings): + continue + findings.append((line_no, line.rstrip())) + return findings + + +def main() -> int: + total = 0 + for path in _enumerate_public_files(): + findings = _check_file(path) + if not findings: + continue + rel = path.relative_to(_REPO_ROOT).as_posix() + for line_no, line_text in findings: + print(f" ✗ {rel}:{line_no} {line_text[:200]}") + total += 1 + if total: + print( + f"\n{total} prohibited reference(s) into gitignored working-memory directories.\n" + "Fix: rewrite the surrounding text to NOT cite docs/marketing/ or docs/analysis/.\n" + "If the reference is a functional path filter (not a content citation), add the\n" + "file path to ``_EXEMPT`` in tools/check_no_analysis_refs.py with a written\n" + "justification. See docs/standards/documentation.md ('Working-memory directories')." + ) + return 1 + print("OK: no public-tree references into docs/marketing/ or docs/analysis/.") + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/tools/check_wizard_defaults_sync.py b/tools/check_wizard_defaults_sync.py new file mode 100644 index 00000000..55d8af9b --- /dev/null +++ b/tools/check_wizard_defaults_sync.py @@ -0,0 +1,104 @@ +"""CI guard — wizard defaults JSON / JS shipped artefacts match the schema. + +Re-runs :mod:`tools.generate_wizard_defaults` into a temp directory and +diffs the output against the committed files. Fails the run with a +clear message + a one-liner the operator can copy-paste to regenerate. + +Why a guard instead of always-regenerate? + +The shipped JSON / JS files are operator-facing artefacts (one is +package data ``forgelm/wizard/_defaults.json``, the other is web asset +``site/js/wizard_defaults.js``). Always regenerating at install time +would either (a) require a build step the project doesn't have, or +(b) leave the operator's working tree dirty after every test run. +Treating the artefacts as committed source + verifying drift at PR +time is the same discipline the project uses for SBOMs and audit-event +catalogues. + +Run via:: + + python tools/check_wizard_defaults_sync.py +""" + +from __future__ import annotations + +import sys +from pathlib import Path + +_REPO_ROOT = Path(__file__).resolve().parent.parent +sys.path.insert(0, str(_REPO_ROOT)) + +from tools.generate_wizard_defaults import ( # noqa: E402 -- after path tweak + JS_TARGET, + PYTHON_TARGET, + collect_defaults, + render_js_literal, + render_python_json, +) + + +def _display_path(target: Path) -> str: + """Best-effort relative path for display; falls back to absolute. + + The guard normally runs against in-repo paths; tests that + monkeypatch ``PYTHON_TARGET`` to a tmp dir would crash inside + ``Path.relative_to`` without this guard. + """ + try: + return str(target.relative_to(_REPO_ROOT)) + except ValueError: + return str(target) + + +def _check_one(target: Path, expected: str) -> bool: + """Return ``True`` when *target* matches *expected* byte-for-byte.""" + if not target.is_file(): + print(f" ✗ {_display_path(target)} is missing.") + return False + actual = target.read_text(encoding="utf-8") + if actual == expected: + return True + print(f" ✗ {_display_path(target)} drifted from schema-derived value.") + # Print the first ~10 differing lines so operators see WHAT + # changed without having to scroll the whole file. + actual_lines = actual.splitlines() + expected_lines = expected.splitlines() + diffs_shown = 0 + for i, (a, e) in enumerate(zip(actual_lines, expected_lines)): + if a != e: + print(f" line {i + 1}:") + print(f" on disk: {a}") + print(f" schema : {e}") + diffs_shown += 1 + if diffs_shown >= 5: + print(" ...(truncated; regenerate to see the full diff)") + break + return False + + +def main() -> int: + defaults = collect_defaults() + expected_python = render_python_json(defaults) + expected_js = render_js_literal(defaults) + + ok_python = _check_one(PYTHON_TARGET, expected_python) + ok_js = _check_one(JS_TARGET, expected_js) + + if ok_python and ok_js: + section_count = len(defaults) + field_count = sum(len(v) for v in defaults.values()) + print(f"OK: shipped wizard defaults ({section_count} section(s) / {field_count} field(s)) match the schema.") + return 0 + + print( + "\nFix: a wizard-flagged Pydantic field's default changed without " + "regenerating the shipped artefacts. Run:\n" + " python tools/generate_wizard_defaults.py\n" + "and commit the updated forgelm/wizard/_defaults.json + " + "site/js/wizard_defaults.js." + ) + return 1 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/tools/check_yaml_snippets.py b/tools/check_yaml_snippets.py index eb2ec5b7..e1966ab9 100644 --- a/tools/check_yaml_snippets.py +++ b/tools/check_yaml_snippets.py @@ -17,8 +17,9 @@ missing required fields, and the doc author intentionally elided them for readability. -Path-level scope: ``docs/analysis/`` is skipped (gitignored research -artefacts; not user-facing). +Path-level scope: gitignored working-memory directories are skipped +(see ``_SKIP_PATH_FRAGMENTS`` for the canonical list — those paths +carry local-only artefacts that aren't user-facing). Exit codes (per ``tools/`` contract — NOT the public 0/1/2/3/4 surface that ``forgelm/`` honours): @@ -83,7 +84,9 @@ # validated against ForgeConfig without inventing the missing keys. _REQUIRED_FORGELM_KEYS = frozenset({"model", "training", "data"}) -# Paths to skip entirely (gitignored research artefacts). +# Paths to skip entirely — gitignored working-memory directories carry +# local-only YAML examples that aren't part of the user-facing surface +# the guard validates against ``ForgeConfig``. _SKIP_PATH_FRAGMENTS = ("docs/analysis/", "docs/marketing/") diff --git a/tools/generate_wizard_defaults.py b/tools/generate_wizard_defaults.py new file mode 100644 index 00000000..2143cfeb --- /dev/null +++ b/tools/generate_wizard_defaults.py @@ -0,0 +1,229 @@ +"""Schema-driven wizard defaults generator (F1 / review-cycle 3). + +Walks every Pydantic submodel reachable from :class:`forgelm.config.ForgeConfig`, +finds fields marked with ``json_schema_extra={"wizard": True}``, extracts +the schema default, and writes two artefacts: + +1. ``forgelm/wizard/_defaults.json`` — Python consumer; loaded via + :mod:`importlib.resources` from :func:`forgelm.wizard._state._load_defaults`. +2. ``site/js/wizard_defaults.js`` — Web consumer; emitted as a tiny script + that sets ``window.WIZARD_DEFAULTS = {...}`` so :file:`site/js/wizard.js`'s + ``defaultState()`` can read schema-aligned values without a build step. + +Run after any schema default change:: + + python tools/generate_wizard_defaults.py + +The CI guard ``tools/check_wizard_defaults_sync.py`` re-runs this script +into a temp dir and diffs against the committed files; PRs that change +a wizard-flagged default without regenerating fail the doc-guard step. + +Why a generator instead of importing live values at runtime? + +- The web wizard is a static site (no Python runtime to query schema + during page load) — it needs JSON-as-data shipped with the assets. +- The Python side benefits too: the wizard module's import-time cost + stays low (no Pydantic model walk), and the JSON is auditable in git. +- Round-tripping through JSON forces all wizard-relevant defaults to be + JSON-serialisable primitives — no datetimes, no Path, no ``Field``s + leaking into the operator's YAML. +""" + +from __future__ import annotations + +import json +import sys +import typing +from pathlib import Path +from typing import Any, Dict, Optional, Tuple, Type + +# Allow `python tools/generate_wizard_defaults.py` from the repo root +# without a separate ``python -m forgelm.tools`` install. +_REPO_ROOT = Path(__file__).resolve().parent.parent +sys.path.insert(0, str(_REPO_ROOT)) + +from pydantic import BaseModel # noqa: E402 -- after path tweak +from pydantic_core import PydanticUndefined # noqa: E402 -- after path tweak + +from forgelm.config import ForgeConfig # noqa: E402 -- after path tweak + +# --------------------------------------------------------------------------- +# Wizard-flag contract +# +# Fields carrying ``json_schema_extra={"wizard": True}`` MUST have +# JSON-primitive defaults (int / float / bool / str / None / list of +# primitives / dict of primitives). ``Path``, ``datetime``, callables, +# and arbitrary BaseModel instances will fail ``json.dumps`` and bring +# down both the generator and the CI guard. This contract is enforced +# implicitly by the JSON serialisation step at the end of ``main()``; +# document it here so future contributors don't get surprised. +# +# Optional[BaseModel] sub-blocks are unwrapped via :mod:`typing` so a +# wizard flag on, say, ``evaluation.cost_ceiling_usd`` (where +# ``evaluation: Optional[EvaluationConfig]``) is picked up correctly +# rather than silently skipped (B1 fix from review-cycle 3). +# --------------------------------------------------------------------------- + +# Output destinations — both kept in lockstep by the CI guard. +PYTHON_TARGET = _REPO_ROOT / "forgelm" / "wizard" / "_defaults.json" +JS_TARGET = _REPO_ROOT / "site" / "js" / "wizard_defaults.js" + +# Stable section order in the output JSON / JS — matches the order +# fields appear inside ForgeConfig so visual diffs read naturally. +_SECTION_ORDER: Tuple[str, ...] = ( + "model", + "lora", + "training", + "data", +) + + +def _is_wizard_flagged(field_info: Any) -> bool: + """Return True when *field_info* carries ``json_schema_extra={"wizard": True}``. + + Pydantic v2 stores the extra dict on ``FieldInfo.json_schema_extra``; + operators may also set it via ``Field(..., wizard=True)`` shortcut + that lands as a nested ``"extra": {...}`` mapping in older releases. + Cover both shapes defensively. + """ + extra = getattr(field_info, "json_schema_extra", None) + if isinstance(extra, dict): + return bool(extra.get("wizard")) + return False + + +def _unwrap_basemodel(annotation: Any) -> Optional[Type[BaseModel]]: + """Return the underlying :class:`BaseModel` subclass, or ``None``. + + Handles three shapes: + - bare class: ``EvaluationConfig`` → ``EvaluationConfig`` + - ``Optional[Cls]`` (== ``Union[Cls, None]``) → ``Cls`` + - ``Union[Cls, OtherCls]`` → first ``BaseModel`` found + + B1 fix (review-cycle 3): the previous version only matched the + bare-class case, so ``Optional[XxxConfig]`` sub-blocks (10 of 14 + top-level ForgeConfig fields) were silently skipped — a wizard + flag on any field inside one of them would never reach the JSON. + """ + if isinstance(annotation, type) and issubclass(annotation, BaseModel): + return annotation + args = typing.get_args(annotation) + if not args: + return None + for arg in args: + if isinstance(arg, type) and issubclass(arg, BaseModel): + return arg + return None + + +def _walk_model(model: Type[BaseModel], section: str, sink: Dict[str, Dict[str, Any]]) -> None: + """Populate *sink* with wizard-flagged defaults from *model*. + + Recurses into nested ``BaseModel`` types so a wizard flag on a deep + field still surfaces — including ``Optional[XxxConfig]`` (B1). + ``model_fields`` is Pydantic v2's introspection API; we read each + field's ``default`` attribute directly because the live Pydantic + instance would substitute :data:`pydantic_core.PydanticUndefined` + for required fields (which we skip via identity comparison — B2). + """ + for name, field_info in model.model_fields.items(): + nested = _unwrap_basemodel(field_info.annotation) + if nested is not None: + # Surface the nested submodel under its own section name + # rather than the parent's; this keeps the JSON shape + # operator-readable (``evaluation.safety_eval``, not + # ``evaluation.evaluation.safety_eval``). + _walk_model(nested, name, sink) + continue + if not _is_wizard_flagged(field_info): + continue + default = field_info.default + # B2: identity comparison against the canonical sentinel — + # ``repr().startswith()`` was brittle across Pydantic v2 minor + # releases. Required fields (no default) carry the sentinel; + # skip them silently because wizard-flagged fields should + # always carry a real default. + if default is PydanticUndefined: + continue + sink.setdefault(section, {})[name] = default + + +def collect_defaults() -> Dict[str, Dict[str, Any]]: + """Walk ForgeConfig + every nested submodel; return wizard-flagged defaults.""" + sink: Dict[str, Dict[str, Any]] = {} + for top_name, field_info in ForgeConfig.model_fields.items(): + nested = _unwrap_basemodel(field_info.annotation) + if nested is not None: + _walk_model(nested, top_name, sink) + # Reorder for deterministic output (sections that aren't in + # ``_SECTION_ORDER`` come last, alphabetised for repeatability). + ordered: Dict[str, Dict[str, Any]] = {} + for section in _SECTION_ORDER: + if section in sink: + ordered[section] = dict(sorted(sink[section].items())) + for section in sorted(sink): + if section not in ordered: + ordered[section] = dict(sorted(sink[section].items())) + return ordered + + +_HEADER_PYTHON = ( + "{\n" + ' "//": "Schema-derived wizard defaults — DO NOT EDIT BY HAND.",\n' + ' "//1": "Regenerate via: python tools/generate_wizard_defaults.py",\n' + ' "//2": "CI guard: tools/check_wizard_defaults_sync.py rejects manual drift.",\n' +) + + +def render_python_json(defaults: Dict[str, Dict[str, Any]]) -> str: + """JSON output with a leading ``//`` comment block for human readers. + + The comment lines use the YAML-style ``"//"`` key idiom — JSON itself + has no comment syntax, but Python's ``json.load`` happily parses + string keys named ``"//"`` and the wizard ignores them when + consuming the file (only specific section keys are read). + """ + body = json.dumps(defaults, indent=2, ensure_ascii=False) + # Strip the opening brace from ``body`` and prepend our header so + # the comments precede the real keys. Defaults dict is non-empty + # so ``body`` always starts with ``{\n``. + payload = body[2:] # drop the leading "{\n" + return _HEADER_PYTHON + payload + "\n" + + +def render_js_literal(defaults: Dict[str, Dict[str, Any]]) -> str: + """Render as a tiny JS asset that exposes ``window.WIZARD_DEFAULTS``.""" + body = json.dumps(defaults, indent=2, ensure_ascii=False) + return ( + "/**\n" + " * Schema-derived wizard defaults — DO NOT EDIT BY HAND.\n" + " * Regenerate via: python tools/generate_wizard_defaults.py\n" + " * CI guard: tools/check_wizard_defaults_sync.py rejects manual drift.\n" + " *\n" + " * Consumed by site/js/wizard.js's defaultState() so the web\n" + " * wizard's accept-all-defaults YAML matches ForgeConfig() byte-\n" + " * for-byte. Loaded BEFORE wizard.js in the HTML pages that\n" + " * mount the wizard modal.\n" + " */\n" + f"window.WIZARD_DEFAULTS = {body};\n" + ) + + +def main() -> int: + defaults = collect_defaults() + PYTHON_TARGET.parent.mkdir(parents=True, exist_ok=True) + JS_TARGET.parent.mkdir(parents=True, exist_ok=True) + PYTHON_TARGET.write_text(render_python_json(defaults), encoding="utf-8") + JS_TARGET.write_text(render_js_literal(defaults), encoding="utf-8") + section_count = len(defaults) + field_count = sum(len(v) for v in defaults.values()) + print( + f"Wrote {section_count} section(s) / {field_count} wizard-flagged default(s):" + f"\n - {PYTHON_TARGET.relative_to(_REPO_ROOT)}" + f"\n - {JS_TARGET.relative_to(_REPO_ROOT)}" + ) + return 0 + + +if __name__ == "__main__": + sys.exit(main())