[Wave 3] Medium — Module sweep: Trainer core, OOM recovery, auto-revert & GRPO rewards (part 1/2) #569

Wave 3 · 21 finding(s) in 20 fix package(s). Fix these whenever a pull request touches this module ("clean the module you touch").

Findings

• CR-A02-030 (code, Medium) — Webhook run_name is not the audit run_id, and approve/reject use yet another run_name
  severity Medium · reachability default · intent contract-violation · confidence confirmed · independently confirmed · reproduced end to end
  Failure scenario. Two staged runs of the same model share run_name 'tiny-qwen_finetune'. Their approve pings arrive as run_name 'out' (or whatever the output dir is called). A Slack approval workflow cannot match decision to request.
  Where. forgelm/trainer.py:347 (confirmed), forgelm/cli/subcommands/_approve.py:633 (confirmed), forgelm/cli/subcommands/_approve.py:731, docs/reference/webhook_schema.md:81
  Recommended fix. Carry the audit run_id in every webhook payload as run_name or as an allowlisted extra. Make approve/reject use the same identifier as the staging event.

• ↳ same root cause: DOC-D1B04-073 (docs, Low) — docs/reference/webhook_schema.md: false claim — run_name: The run id for training.* and approval.required ... Primary correlation key. Join on run_name and the event name to correlate with the audit…
  severity Low
  Where. docs/reference/webhook_schema.md:81 (also :51) · type: false claim
  The document says. run_name: The run id for training.* and approval.required ... Primary correlation key. Join on run_name and the event name to correlate with the audit log.
  What the code does. run_name is '<basename(model.name_or_path)>_finetune' for trainer events and basename(output_dir) for approve/reject. It is never the audit run_id and does not occur in audit_log.jsonl.
  Code references. forgelm/trainer.py:347, forgelm/cli/subcommands/_approve.py:633,731
  Proposed correction. Document the real derivation and point to model_path's staging suffix, or fix the code (finding F1).
  Turkish mirror. the Turkish page has the same error; fix both in the same PR

• CR-A02-032 (code, Medium) — _revert_model reports the model as deleted even when shutil.rmtree failed
  severity Medium · reachability documented-opt-in · intent intended-but-harmful · confidence confirmed · independently confirmed · reproduced with stubs
  Failure scenario. A permission or busy-file error interrupts the revert. Operators, dashboards and the audit trail all say the rejected model was discarded, but a partially deleted model directory remains at the canonical final_model path.
  Where. forgelm/trainer.py:1007 (notify_reverted reason always ends 'Artifacts discarded.'), forgelm/trainer.py:1634-1639, forgelm/trainer.py:1354-1373, docs/usermanuals/en/evaluation/auto-revert.md:104-106
  Recommended fix. Have _revert_model return whether the deletion succeeded. Emit a follow-up audit event (e.g. a correction or deletion-failed record) and keep final_model_path or state 'deletion failed' in the result and webhook when rmtree fails.

• CR-A02-035 (code, Medium) — With lora.method: pissa the auto-measured baseline loss is that of the residual model, not the base model, loosening the regression gate
  severity Medium · reachability default · intent contract-violation · confidence confirmed · independently confirmed · reproduced end to end
  Failure scenario. PiSSA fine-tune whose eval_loss is worse than the original base model but better than the residual-model baseline passes the baseline regression gate under auto_revert: true.
  Where. forgelm/trainer.py:1288-1302, forgelm/model.py:539-540, forgelm/trainer.py:892-893, forgelm/trainer.py:1325
  Recommended fix. Measure the baseline with adapters enabled before training (equivalent to the base model at init for every supported method), or skip disable_adapter when init_lora_weights is not the default.

• CR-A02-036 (code, Medium) — OOM recovery frees memory and rebuilds the trainer while the failed trainer, its traceback and the GRPO reward model are still referenced
  severity Medium · reachability documented-opt-in · intent contract-violation · confidence confirmed · independently confirmed · reproduced with stubs
  Failure scenario. GRPO with a 7B reward model on one GPU hits OOM; the recovery loads a second 7B reward model next to the first and crashes with a fresh OOM inside _build_trainer instead of retrying at the smaller batch.
  Where. forgelm/trainer.py:1267-1268, forgelm/trainer.py:1071 (confirmed), forgelm/trainer.py:1168-1174 (confirmed), forgelm/trainer.py:1229-1270
  Recommended fix. Build the reward callables once and reuse them on rebuild (cache on self), and drop self.trainer (self.trainer = None; gc.collect(); empty_cache) before rebuilding.

• CR-A02-037 (code, Medium) — merge_adapters failure silently degrades to an unmerged (possibly empty-adapter) save and the run succeeds
  severity Medium · reachability documented-opt-in · intent intended-but-harmful · confidence confirmed · independently confirmed · reproduced with stubs · already tracked: KNOWN-OPEN C31-RUN-OUTCOME — docs/roadmap/phase-16-trust-surface-hardening.md:543-544 (S8 open)
  Failure scenario. merged.save_pretrained raises (e.g. a safetensors ValueError); the fallback silently ships an empty adapter, gates pass on the in-memory model, exit 0.
  Where. forgelm/trainer.py:1733-1749 (confirmed), forgelm/trainer.py:1735-1750, tests/test_trainer.py:1069-1079, forgelm/config.py:457-460
  Recommended fix. Only fall back when merge_and_unload itself failed; if the merged save fails, raise (or save the merged model via trainer.save_model). Record merged/unmerged in the manifest and model card.

• CR-A02-038 (code, Medium) — oom_recovery is not guarded against distributed strategies (DeepSpeed/FSDP/multi-rank): the retry is rank-local and rebuilds a trainer on an already-wrapped model
  severity Medium · reachability documented-opt-in · intent intended-but-harmful · confidence confirmed · independently confirmed · not reproduced (code trace)
  Failure scenario. A 2-node FSDP job with oom_recovery=true. Rank 3 OOMs on a long batch and retries at bs/2. Ranks 0-2 and 4-7 block in all-gather. After the NCCL timeout the whole job aborts with a watchdog error instead of the clear OOM, and the operator has paid the wait.
  Where. forgelm/trainer.py:1232-1270, forgelm/trainer.py:591-615, forgelm/config.py:598-603, forgelm/config.py:1937-1953
  Recommended fix. Either reject oom_recovery: true when distributed.strategy is set (config error with a hint), or make the recovery collective: all-reduce an OOM flag across ranks, and tear down and re-create the accelerator or engine. Rejecting it is the cheap and honest option. Document the combination until then. Expected gain: avoids 30-minute stalls on multi-GPU failures.

• CR-A02-040 (code, Medium) — GRPO classifier reward: device_map='auto' under distributed training, unchunked scoring and 512-token truncation
  severity Medium · reachability documented-opt-in · intent intended-but-harmful · confidence confirmed · independently confirmed · not reproduced (code trace)
  Failure scenario. An 8-GPU FSDP GRPO run with a 7B reward model. Each rank requests the whole model with device_map=auto, which fills all GPUs and leaves no room for the policy shards.
  Where. forgelm/trainer.py:1176-1183, forgelm/model.py:406-412, forgelm/trainer.py:1024, forgelm/trainer.py:1071
  Recommended fix. Skip device_map under distributed (mirror _device_map_for), place the reward model on the rank's local device, score in micro-batches of a few completions, and make max_length follow grpo_max_completion_length plus the prompt cap. Expected gain: removes (N-1) reward-model copies and bounds the scoring peak.

• CR-A02-041 (code, Medium) — The post-training pipeline wiring in _run_training_pipeline/train() is effectively untested: 20+ behaviour-destroying mutations survive
  severity Medium · reachability default · intent contract-violation · confidence confirmed · independently confirmed · reproduced end to end
  Failure scenario. A refactor drops return train_result after a safety/judge/benchmark revert, or after the approval gate. The pipeline then writes model card, integrity and compliance bundle into a deleted directory, or marks an awaiting-approval run as completed with a success webhook. Every existing test stays green.
  Where. forgelm/trainer.py:1571-1659, forgelm/trainer.py:1587-1588, forgelm/trainer.py:1604-1620, forgelm/trainer.py:1641-1655
  Recommended fix. Add pipeline-level tests that run the real _run_training_pipeline with a stub HF trainer (real audit logger, tmp_path, real save path via a tiny saver) for: require_human_approval (staging dir name carries run_id, final_model absent, human_approval.required event, no pipeline.completed, awaiting_approval); each gate failure with auto_revert on/off (model dir deleted or kept, later stages not run,…

• CR-A02-042 (code, Medium) — training.packing on trl 1.x uses padding-free BFD packing without Flash Attention; trl warns of cross-sample contamination while the docs promise unchanged quality
  severity Medium · reachability documented-opt-in · intent contract-violation · confidence confirmed · independently confirmed · reproduced end to end
  Failure scenario. An operator enables packing: true as the long-context guide recommends ('critical for throughput'). Training completes with exit 0, but samples attend across boundaries, which silently degrades the model.
  Where. forgelm/trainer.py:693, forgelm/config.py:499, docs/usermanuals/en/training/long-context.md:94
  Recommended fix. When packing is on, require or set a Flash-Attention attn_implementation, or select trl's wrapped strategy, and refuse or warn at config load. Correct the manual's claim.

• CR-A02-043 (code, Medium) — Gates generate on a model left in train mode (dropout, gradient checkpointing) when no post-train evaluate() ran
  severity Medium · reachability default · intent contract-violation · confidence confirmed · independently confirmed · reproduced end to end
  Failure scenario. A run with a 1-row dataset (no validation split) and lora.dropout 0.1: at judge time model.training is True with 4 active Dropout modules.
  Where. forgelm/trainer.py:1585-1588 (evaluate() guard; the line numbers cited as 1587-1588 are correct), forgelm/trainer.py:1641-1649 (gate calls), forgelm/judge.py:391-392 (_generate_response) and :428-429 (batched): affected, forgelm/trainer.py:514 (gradient_checkpointing = torch.cuda.is_available()); forgelm/trainer.py:599 (forced True under DeepSpeed)
  Recommended fix. Call model.eval() (and restore the previous mode afterwards) before the post-training gates.

• CR-A02-046 (code, Medium) — Interrupted runs (Ctrl-C, or SIGTERM from torchrun/k8s/Slurm preemption) leave the lifecycle unterminated: no pipeline.failed, no failure webhook, no JSON envelope, and an exit code outside the contr…
  severity Medium · reachability default · intent contract-violation · confidence confirmed · independently confirmed · reproduced end to end
  Failure scenario. A Kubernetes job running training is preempted (SIGTERM). The Art. 12 log shows a run that started and never ended, the Slack dashboard shows the run in progress forever, and CI sees exit 143, which is not in the 0-6 contract.
  Where. forgelm/trainer.py:1693-1708, forgelm/trainer.py:1669, forgelm/cli/_training.py:173, forgelm/cli/_dispatch.py:166-169
  Recommended fix. In ForgeTrainer.train and the orchestrator, also catch KeyboardInterrupt (and SystemExit raised from a SIGTERM handler): emit pipeline.failed/pipeline.interrupted, call notify_failure, then re-raise. Install a SIGTERM handler at the CLI seam that raises a custom exception so this path runs, and emit the JSON envelope and a log line on interrupt.

• CR-A02-047 (code, Medium) — save_final_model writes into an existing final_model without clearing it, mixing runs
  severity Medium · reachability default · intent intended-but-harmful · confidence confirmed · independently confirmed · reproduced end to end
  Failure scenario. An operator resumes training to add steps, and the job is preempted during the safety evaluation. final_model/ now holds unevaluated weights with the old model card and integrity manifest (verify-integrity 6), and the previously validated model is gone.
  Where. forgelm/trainer.py:1619-1620 (confirmed), forgelm/trainer.py:1711-1740, forgelm/trainer.py:1652, forgelm/trainer.py:995-998
  Recommended fix. Always save to a per-run staging directory and promote it to final_model/ only after the gates and finalization (atomic rename, moving the previous final_model aside). Revert then only removes the staging dir.

• CR-A02-048 (code, Medium) — baseline_loss is auto-computed only when auto_revert is true, contrary to the configuration reference
  severity Medium · reachability default · intent contract-violation · confidence confirmed · independently confirmed · reproduced end to end
  Failure scenario. An operator relying on the documented automatic baseline, with auto_revert left false, gets no regression check. The audit event records baseline_loss: null.
  Where. forgelm/trainer.py:1282-1285, forgelm/config.py:1209-1214, docs/reference/configuration.md:186, forgelm/trainer.py:1576
  Recommended fix. Document the auto_revert dependency in configuration.md:186 (EN+TR) and the field description, or compute the baseline whenever a validation split exists.

• CR-A02-050 (code, Medium) — A trailing slash in training.final_model_dir stages the model inside final_model/ and makes forgelm approve impossible
  severity Medium · reachability documented-opt-in · intent contract-violation · confidence confirmed · independently confirmed · reproduced end to end
  Failure scenario. An operator writes final_model_dir: "final_model/" in YAML (TC-T, IN-01) with require_human_approval: true. Training exits 4, but final_model/ already exists on disk before approval: a watcher that keys on the directory sees it, although it holds only a hidden subdir. The documented approve command fails with exit 1 and advises deleting final_model/, which would delete the staged model. The run can never be promoted through the gate.
  Where. forgelm/trainer.py:1590-1593, forgelm/trainer.py:1611, forgelm/config.py:454-456, forgelm/cli/subcommands/_approve.py:555-556
  Recommended fix. Validate final_model_dir in TrainingConfig: strip trailing separators and reject empty, '.', '..'-bearing or absolute values. Alternatively build the staging path from os.path.normpath(final_path). Make approve's error message not advise deleting a directory that contains the staging dir. Add a regression test for final_model_dir='final_model/'.

• CR-A02-051 (code, Medium) — A library caller who reuses one ForgeConfig across ForgeTrainer runs inherits the previous run's baseline_loss and OOM-reduced batch size: the loss gate compares against a stale baseline (a false aut…
  severity Medium · reachability undocumented · intent contract-violation · confidence confirmed · independently confirmed · reproduced end to end · already tracked: KNOWN-OPEN TRUST-08 — docs/roadmap/phase-16-trust-surface-hardening.md:542-543 (S8 open)
  Failure scenario. In a notebook sweep, a data scientist calls ForgeTrainer(..., config=cfg, ...) for several dataset variants, changing only cfg.data. From the second run on, every regression gate uses the first run's base-model loss on the first dataset. Good models are deleted, or regressed models are promoted, depending on which dataset has the higher base loss. If the first run hit an OOM, every later Annex IV manifest reports the reduced batch size as the configured value.
  Where. forgelm/trainer.py:343, forgelm/trainer.py:1283-1285, forgelm/trainer.py:1283, forgelm/trainer.py:1325
  Recommended fix. Deep-copy the config in ForgeTrainer.init, or keep run state (measured baseline, effective batch size and grad-accum) on the trainer instead of the config. Capture the configured batch values in init from that copy. Add a test that builds two trainers from one ForgeConfig and asserts the second measures its own baseline and records the operator's batch size.

• CR-A02-052 (code, Medium) — No test asserts exit code, TrainResult.success or the JSON envelope for the safety evaluation_completed=False path or for a kept NaN/threshold breach with auto_revert:false
  severity Medium · reachability default · intent contract-violation · confidence confirmed · independently confirmed · reproduced end to end · already tracked: CLAIMED-FIXED-STILL-PRESENT TRUST-03 — docs/roadmap/phase-16-trust-surface-hardening.md:356-366 (S3 checkbox open, delivered by cab2947); CHANGELOG.md:55-63
  Failure scenario. A kept gate (safety evidence unreadable, or loss NaN/over the ceiling with auto_revert:false) flows to exit 0, success:true, pipeline.completed. Any change in either direction (fixing it to exit 3, or breaking it further) leaves the whole suite green because no test crosses TrainResult.success to the exit code or envelope; the one test that asserts success True for a kept gate would hide the defect as 'expected'.
  Where. tests/test_trainer.py:188-214, tests/test_trainer.py:286-308, tests/test_trainer.py:620-651, tests/test_trainer.py:750-763
  Recommended fix. Add an end-to-end style test that drives _run_training_pipeline (or train()) with a scripted safety result with evaluation_completed=False and with auto_revert false plus NaN/over-threshold loss, then calls _training._run_training_pipeline and asserts exit code, result.success and the --output-format json envelope.

• CR-A02-053 (code, Medium) — No test round-trips a freshly trained final_model through verify-integrity
  severity Medium · reachability default · intent contract-violation · confidence confirmed · independently confirmed · reproduced end to end
  Failure scenario. Every freshly trained model fails forgelm verify-integrity (exit 6, the security-event code) because the manifest is written before deployer_instructions.md (trainer.py:1504-1505) and the verifier flags any unrecorded file (_model_integrity.py:199-210). The suite stays green because the writer order and the verifier are never exercised together.
  Where. tests/test_verification_toolbelt.py:1089-1099 (helper _write_model_with_integrity; class TestVerifyIntegrity starts at 1101), tests/test_integration.py:314-335 (class TestModelIntegrityIntegration), tests/test_eu_ai_act.py:360-413, tests/test_oom_recovery.py:76-77 (both writers mocked)
  Recommended fix. Add a test that runs the real _finalize_artifacts (model card, integrity, deployer instructions) on a tmp final_model and then verify_integrity / the verify-integrity subcommand, asserting exit 0.

• CR-A02-054 (code, Medium) — Docs disagree with each other and with the code on what a kept gate failure does
  severity Medium · reachability default · intent contract-violation · confidence confirmed · independently confirmed · reproduced end to end
  Failure scenario. An operator reading error-handling.md:254, or the usermanual's configuration.md:136, concludes that a failed gate either always exits 3 or does nothing without auto_revert. Neither is true.
  Where. docs/standards/error-handling.md:254, docs/standards/error-handling.md:22, docs/usermanuals/en/reference/configuration.md:136, docs/usermanuals/en/reference/exit-codes.md:17
  Recommended fix. Qualify error-handling.md:254 by auto_revert; fix configuration.md:136 (EN+TR); add the loss gate to exit-codes.md and error-handling.md; mention the require_human_approval → exit 4 case.

• CR-A02-059 (code, Medium) — Mixed-precision selection counts emulated bf16 as native, so T4/V100 run emulated bf16 and the intended fp16 branch never runs
  severity Medium · reachability default · intent intended-but-harmful · confidence confirmed · independently confirmed · not reproduced (code trace)
  Where. forgelm/trainer.py:517, forgelm/trainer.py:516, forgelm/model.py:247, forgelm/model.py:444
  Recommended fix. Make the decision on native support: torch.cuda.is_bf16_supported(including_emulation=False), or torch.cuda.get_device_capability()[0] >= 8, at all four sites. Better, add an explicit training.mixed_precision field (auto|bf16|fp16|no). Fixing this alone makes the fp16 branch live and exposes the latent defects in the fp16-lane finding (lora.bias), so handle those in the same change.

• CR-A02-060 (code, Medium) — Mixed precision cannot be configured, yet the troubleshooting guides tell users to set bf16/fp16 fields the schema rejects
  severity Medium · reachability default · intent contract-violation · confidence confirmed · independently confirmed · reproduced end to end · already tracked: KNOWN-OPEN C31-DOC-SCHEMA-PROSE — docs/roadmap/phase-16-trust-surface-hardening.md:785-786 (S15 open)
  Where. docs/guides/troubleshooting.md:109, docs/guides/troubleshooting-tr.md:111, docs/usermanuals/en/operations/troubleshooting.md:49, docs/usermanuals/en/operations/troubleshooting.md:75
  Recommended fix. Either add a training.mixed_precision field, which also fixes the emulated-bf16 lane, or remove that advice and state the automatic policy.

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.}