Repository navigation
[Wave 3] Medium — Module sweep: Onboarding: quickstart templates and the CLI wizard (part 2/2) #563
Description
Wave 3 · 4 finding(s) in 4 fix package(s). Fix these whenever a pull request touches this module ("clean the module you touch").
Findings
-
CR-A14-029 (code, Medium) — --wizard-start-from cannot keep any non-preset lora.target_modules: an Enter-only rerun rewrites a correct custom list (e.g. Phi-3 qkv_proj/gate_up_proj, or ['all-linear']) to [q_proj, v_proj], contr…
severity Medium · reachabilitydefault· intentcontract-violation· confidence confirmed · independently confirmed · reproduced end to end · already tracked: KNOWN-OPEN CLI-09 — docs/roadmap/phase-16-trust-surface-hardening.md:502-503,512-513 (S7 open; unit id inferred)
Failure scenario. A rerun of a working custom-target config with Enter at every prompt exits 0. The rewritten YAML, saved over the original by default according to the guide, now fails at model load with exit 2, or on other architectures silently trains a different adapter scope.
Where.forgelm/wizard/_orchestrator.py:382,forgelm/wizard/_orchestrator.py:415-426,docs/guides/quickstart.md:80,docs/guides/quickstart-tr.md:85
Recommended fix. When the loaded list matches no preset, add a fourth option 'keep existing ()', make it the default, and leave lora.target_modules unchanged when it is chosen. Add a test next to test_wizard_phase22.py:1746 for a non-preset list. -
CR-A14-030 (code, Medium) — Wizard step 1 silently writes model.backend: unsloth on any Linux+GPU host, with no prompt, no extra check and no compatibility check
severity Medium · reachabilitydefault· intentintended-but-harmful· confidence confirmed · independently confirmed · reproduced end to end
Failure scenario. An operator on a Linux CUDA box withpip install forgelm(or the default Docker image) runsforgelm --wizard, accepts the defaults and answers 'Start training now?' with y, or later runsforgelm --config my_config.yaml --dry-run(exit 0) and then trains. The run exits 2 at model load with ImportError, after the wizard and the dry-run both certified the config.
Where.forgelm/wizard/_orchestrator.py:223-235(confirmed),forgelm/wizard/_io.py:209-227,forgelm/wizard/_orchestrator.py:1090-1094,forgelm/wizard/_orchestrator.py:1155-1196
Recommended fix. In _step_welcome, suggest unsloth only when importlib.util.find_spec('unsloth') is not None. Otherwise keep transformers and print the install hint. Better still, make the backend a confirmed prompt. Add a find_spec check for the configured backend's extra to --dry-run and to the wizard pre-flight checklist (no heavy import is needed). Add a test that monkeypatches GPU+linux and a missing find_sp… -
CR-A14-031 (code, Medium) — Validate-on-exit and the start-from load check only the root ForgeConfig, never the merged pipeline stages, so stage conflicts (including ones created by the wizard's root-only strict-tier coercion)…
severity Medium · reachabilitydefault· intentcontract-violation· confidence confirmed · independently confirmed · reproduced end to end
Failure scenario. Pipeline YAML: root minimal-risk; stage dpo_stage has evaluation: {auto_revert: false}. In step 8 the operator selects high-risk. The wizard coerces root evaluation.safety, saves, prints '✓ Schema validation passed' and offers 'Start training now?'.forgelm --config out.yaml --dry-runexits 1: Stage 'dpo_stage': merge failed: Risk classification 'high-risk' requires evaluation.safety.enabled: true. A real run trains sft_stage first and fails on dpo_stage.
Where.forgelm/wizard/_orchestrator.py:1020-1024(root-only model_validate at load; reported as :1015-1019),forgelm/wizard/_orchestrator.py:1168-1195,forgelm/wizard/_orchestrator.py:91-122,forgelm/wizard/_orchestrator.py:1090-1095
Recommended fix. When a pipeline block is present, run merge_pipeline_stage_config for every stage (as PipelineOrchestrator.dry_run does) in both _load_initial_state_from_yaml and _validate_generated_config. Either make strict-tier coercion stage-aware, or warn that stages with their own evaluation block need safety enabled. Consider making run()'s pre-flight treat merge errors as fatal before any stage trains. -
CR-A14-032 (code, Medium) — Wizard step diff never reports removed keys, so fields (including secrets and endpoints) dropped by start-from collectors vanish with no on-screen warning
severity Medium · reachabilitydefault· intentcontract-violation· confidence confirmed · independently confirmed · reproduced end to end
Failure scenario. An operator reruns the wizard on a production config, answers 'y' at the webhook gate, sees a step diff of three '~' lines and saves. require_https, tls_ca_bundle, allow_private_destinations and timeout are gone, and nothing before the save said so.
Where.forgelm/wizard/_state.py:407-419,forgelm/wizard/_orchestrator.py:869(confirmed),forgelm/wizard/_orchestrator.py:1085-1090,tests/test_wizard_phase22.py:395-418
Recommended fix. Also compute removed = prev_flat.keys() - curr_flat.keys() and print them as '- key: old'. Do not return early when only removals exist. Add a TestStepDiff case for removals.
Done when
- Every finding above is fixed or closed with a reason (tick it here).
- Code fixes carry a test; doc fixes update the Turkish mirror in the same PR.
Found by the October 2026 ForgeLM code and documentation review against commit cab2947. Code links point at that commit; line numbers may have moved since. Finding IDs (CR-…, DOC-…, TECH-…) are stable references for this backlog.
Metadata
Metadata
Assignees
Labels
Type
Projects
- StatusShow more project fieldsTodo
Activity