[Wave 4] Low/info — Module sweep: Trainer core, OOM recovery, auto-revert & GRPO rewards (part 2/4) #709

Wave 4 · 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-101 (code, Low) — Code does not match docs/standards/architecture.md: Modules do not read env vars for decisions (only secrets) and do not mutate config at runtime.
  severity Low · reachability default · intent intended-but-harmful · confidence likely · independently confirmed
  Failure scenario. The document states: Modules do not read env vars for decisions (only secrets) and do not mutate config at runtime.. Observed in the code: Config is mutated at runtime: OOM recovery rewrites config.training.per_device_train_batch_size and gradient_accumulation_steps, and --offline sets config.model.offline. Behavioural env vars are read in library code: FORGELM_ALLOW_ANONYMOUS_OPERATOR, FORGELM_ALLOW_AUDIT_REROOT, FORGELM_GGUF_CONVERTER, and a test hook FORGELM_AUDIT_TEST_WORKER_RAISES in data_…
  Where. forgelm/trainer.py:1264-1265, forgelm/trainer.py:2071-2078, forgelm/cli/_config_load.py:50-52, forgelm/compliance.py:219
  Recommended fix. List the sanctioned exceptions (OOM recovery batch rewrite, offline flag, FORGELM_ALLOW_* safety opt-ins), and move the test hook out of production code.

• ↳ same root cause: DOC-D1B07-019 (docs, Low) — docs/standards/architecture.md: misleading — Modules do not read env vars for decisions (only secrets) and do not mutate config at runtime.
  severity Low
  Where. docs/standards/architecture.md:182-184 · type: misleading
  The document says. Modules do not read env vars for decisions (only secrets) and do not mutate config at runtime.
  What the code does. Config is mutated at runtime: OOM recovery rewrites config.training.per_device_train_batch_size and gradient_accumulation_steps, and --offline sets config.model.offline. Behavioural env vars are read in library code: FORGELM_ALLOW_ANONYMOUS_OPERATOR, FORGELM_ALLOW_AUDIT_REROOT, FORGELM_GGUF_CONVERTER, and a test hook FORGELM_AUDIT_TEST_WORKER_RAISES in data_audit/orchestrator.py.
  Code references. forgelm/trainer.py:1264, forgelm/trainer.py:2065-2078, forgelm/cli/_config_load.py:50-52, forgelm/compliance.py:218,407,427
  Proposed correction. List the sanctioned exceptions (OOM recovery batch rewrite, offline flag, FORGELM_ALLOW* safety opt-ins), and move the test hook out of production code.

• CR-A02-102 (code, Low) — auto-revert is never driven with an unusual final_model_dir; the audit-log-survives invariant is untested, and the stub-the-rmtree tests could not catch the deletion
  severity Low · reachability undocumented · intent contract-violation · confidence confirmed · independently confirmed · reproduced end to end
  Failure scenario. Config training.final_model_dir: '' or '..' (a typo, or a CI-templated or fork-PR config, IN-01 in CI-PR) plus evaluation.auto_revert: true and a failing gate. _revert_model rmtrees output_dir or its parent, destroying the append-only Art. 12 audit log and compliance artefacts, even though the 'model.reverted' event was written just before. The suite stays green because only the default name is ever used.
  Where. forgelm/trainer.py:1590-1593, forgelm/trainer.py:995-997, forgelm/config.py:454-456, tests/test_trainer.py:264
  Recommended fix. Minimal test that would have caught M-A01-002. Parametrise over final_model_dir in ['', '.', '..', '../x', '/abs/outside', 'sub/../..']. Use a real AuditLogger(tmp/out), a real _revert_model, and sentinel files for audit_log.jsonl and a sibling directory. Assert either that config validation rejects the value, or, after the revert, that out/audit_log.jsonl exists and verify_audit_log passes, and…

• CR-A02-103 (code, Low) — No test pins the classifier reward's logit index, prompt inclusion, or truncation length
  severity Low · reachability default · intent intended-but-harmful · confidence confirmed · independently confirmed · reproduced end to end
  Failure scenario. Changing the logit index, adding or removing the prompt, or changing the truncation length from 512 changes the RL reward signal while every test stays green.
  Where. forgelm/trainer.py:1177-1184, tests/test_grpo_reward.py:89-129, tests/test_grpo_reward.py:131-203
  Recommended fix. Use a real tiny sequence-classification model, or a recording tokenizer and model that capture their inputs. Assert the score ordering, that prompt and completion are encoded together, the truncation length, and the logit index for num_labels > 1.

• CR-A02-104 (code, Low) — No test asserts TrainResult.reverted or an audit payload when shutil.rmtree fails in _revert_model
  severity Low · reachability default · intent contract-violation · confidence confirmed · independently confirmed · reproduced (plumbing only)
  Failure scenario. final_model is read-only or locked; rmtree raises OSError. _revert_model logs and continues, the webhook says 'Artifacts discarded', _mark_reverted sets reverted=True/final_model_path=None, exit 3 and a JSON envelope claim a deletion that did not happen while the rejected weights sit on disk. A mutation that makes the except branch set a 'deletion_failed' flag, or removes the false claim, cannot be detected; equally a mutation that leaves behaviour as is passes. No test would fail if the defect…
  Where. tests/test_trainer.py:264-284, tests/test_trainer.py:616, tests/test_human_approval_gate.py:238, forgelm/trainer.py:1007
  Recommended fix. Add a test that makes rmtree fail through the real _revert_model and the real _mark_reverted/apply* chain, then asserts on TrainResult.reverted, final_model_path, the model.reverted payload (which must gain a deleted/deletion_failed field) and the notify_reverted text.

• CR-A02-105 (code, Low) — No OOM-recovery test uses a DeepSpeed, FSDP or multi-rank configuration
  severity Low · reachability documented-opt-in · intent intended-but-harmful · confidence confirmed · independently confirmed · not reproduced (code trace)
  Failure scenario. Under torchrun/accelerate/DeepSpeed, one rank OOMs and rebuilds the trainer with a different batch/accum while peers keep the original, deadlocking collectives or rebuilding on an already-wrapped model. No test would flag removing or adding a distributed guard.
  Where. tests/test_oom_recovery.py:17-33, tests/test_oom_recovery.py:321-415, forgelm/trainer.py:1188-1269, tests/test_oom_recovery.py:430-434
  Recommended fix. Add a test with config.distributed.strategy set (and a WORLD_SIZE>1 env) asserting that OOM recovery either refuses to retry (re-raises with an explicit message) or synchronises ranks, whichever the fix decides.

• CR-A02-106 (code, Low) — The step-0 restart is documented, but the guide calls the result a trajectory that 'differs slightly', and no artefact records how many updates the discarded attempt applied
  severity Low · reachability documented-opt-in · intent intended-but-harmful · confidence likely · single reviewer · reproduced (plumbing only)
  Failure scenario. oom_recovery: true and max_steps: 1000. A CUDA OOM fires at update 900, and the retry trains 1000 more updates on the already-updated weights. trainer_state, the manifest and the audit event all describe a 1000-step run, and the guide describes the difference as slight.
  Where. forgelm/trainer.py:1196-1207, forgelm/trainer.py:1257-1262, docs/guides/troubleshooting.md:64-66, docs/reference/configuration.md:86-108
  Recommended fix. State in troubleshooting.md, and in configuration.md beside the oom_recovery fields, that a retry without a checkpoint re-runs the full schedule on the weights reached so far. Add global_step at the time of the OOM to the training.oom_recovery event, or resume from the latest checkpoint as the docstring already proposes.

• CR-A02-107 (code, Low) — peak_vram_gb is a never-reset high-water mark on device 0, sampled after baseline eval, failed OOM attempts, post-train evaluate and the lm-eval benchmark, so it does not describe the configuration t…
  severity Low · reachability default · intent intended-but-harmful · confidence likely · single reviewer · not reproduced (code trace)
  Failure scenario. On an 80 GB card the run OOMs at batch 16 and recovers at batch 8. The compliance bundle shows peak_vram_gb near 79 next to effective_batch_size 8, although batch 8 needs far less. An operator sizing hardware from resource_usage over-provisions. With benchmark enabled (batch_size auto), peak_vram_gb may reflect lm-eval's batch probe rather than training.
  Where. forgelm/trainer.py:1842, forgelm/trainer.py:1576, forgelm/trainer.py:1579, forgelm/trainer.py:1588
  Recommended fix. Call torch.cuda.reset_peak_memory_stats on the training device before each trainer.train() attempt. Read the peak right after training returns, on the rank's own device or aggregated across devices. Name the field for what it measures (e.g. training_peak_allocated_gb).

• CR-A02-108 (code, Low) — OOM recovery classifies by the substring 'out of memory' in any RuntimeError: unrelated errors trigger restarts and a false training.oom_recovery audit record, while library allocation failures are n…
  severity Low · reachability documented-opt-in · intent intended-but-harmful · confidence likely · independently confirmed · reproduced with stubs
  Failure scenario. A run with oom_recovery: true hits a cuBLAS workspace allocation failure (CUBLAS_STATUS_ALLOC_FAILED) at batch 16 and exits 2 immediately, although halving the batch would have succeeded. Or a non-allocator RuntimeError whose text mentions 'out of memory' causes four restarts from step 0 of GPU time, writes four training.oom_recovery events claiming OOMs that never happened, and ends at exit 2 with a manifest that records a batch-size reduction.
  Where. forgelm/trainer.py:1231-1234, forgelm/trainer.py:1270, forgelm/cli/_training.py:173-180
  Recommended fix. Classify on isinstance(e, torch.OutOfMemoryError) first. Add an explicit allow-list for library allocation statuses (CUBLAS_STATUS_ALLOC_FAILED, CUFFT_ALLOC_FAILED, CUDNN_STATUS_ALLOC_FAILED). Exclude DistBackendError, or guard on world_size==1. Include the matched exception type in the training.oom_recovery event.

• CR-A02-109 (code, Low) — DeepSpeed preset names resolve first from /configs/deepspeed and then, silently, from ./configs/deepspeed in the CWD
  severity Low · reachability documented-opt-in · intent intended-but-harmful · confidence likely · independently confirmed · reproduced end to end
  Failure scenario. A team runs forgelm --config train.yaml from a shared project directory on a multi-user GPU host, with distributed.deepspeed_config: zero2 on a pip-installed ForgeLM. Another user with write access to that directory edits configs/deepspeed/zero2.json to add zero_optimization.offload_param {device: nvme, nvme_path: (local review artefact) or a tensorboard/csv_monitor output_path, or swaps the optimizer. The run uses that file, logs only 'DeepSpeed enabled with config: zero2', and the artefacts…
  Where. forgelm/trainer.py:632-660, forgelm/trainer.py:597, docs/guides/troubleshooting.md:347-352, docs/reference/distributed_training.md:52
  Recommended fix. Resolve presets only from package data (importlib.resources.files('forgelm') / 'deepspeed_presets'). Treat any other file as a custom path the operator must name explicitly, and log plus record its absolute path and sha256. Update troubleshooting.md, which currently tells users to rely on the CWD.

• CR-A02-110 (code, Low) — The DPO manual says a second reference model is kept in VRAM, but with ForgeLM's PEFT model TRL uses the adapter-disabled policy and keeps no reference copy
  severity Low · reachability default · intent contract-violation · confidence likely · single reviewer · not reproduced (code trace)
  Where. docs/usermanuals/en/training/dpo.md:120, docs/usermanuals/en/training/dpo.md:123, docs/usermanuals/en/training/dpo.md:125
  Recommended fix. Describe the PEFT reference mechanism (adapter disabled, one model, extra forward pass) in EN and TR.

• CR-A02-111 (code, Low) — An audit-write failure inside the compliance except clauses replaces the original failure in the envelope and the audit log, and the governance catch misreports an audit failure as a governance-repor…
  severity Low · reachability default · intent intended-but-harmful · confidence likely · independently confirmed · reproduced with stubs
  Failure scenario. Injected: export_compliance_artifacts raises OSError('injected export...'), and log_event raises for compliance.artifacts_export_failed. Result: EXIT=2, stdout {"success": false, "error": "injected audit write failure for compliance.artifacts_export_failed"}, audit [..., pipeline.failed{error: same}]. The export failure appears nowhere except the stderr chained traceback. H2 (governance ValueError plus audit failure on governance_failed and artifacts_export_failed): same envelope; the governanc…
  Where. forgelm/trainer.py:2118-2127, forgelm/trainer.py:2148-2155, forgelm/trainer.py:1693-1709, forgelm/compliance.py:600-606
  Recommended fix. Put the governance report write and its log_event in separate try blocks so an audit failure is not reported as a governance failure. In the outer except, isolate the audit emit (try/except OSError) and re-raise a combined error that keeps the original reason (e.g. raise ... from e, with the message carrying both), so the envelope and pipeline.failed name the root cause.

• CR-A02-112 (code, Low) — No receipt event exists; --compliance-export builds no ZIP and writes no audit event
  severity Low · reachability default · intent contract-violation · confidence likely · independently confirmed · reproduced end to end
  Failure scenario. A deployer following the QMS expects the audit chain to prove the auditor received the bundle (EU AI Act Art. 12 evidence). No such record can be produced, and the standalone export leaves no audit trace at all.
  Where. forgelm/trainer.py:2142-2147, forgelm/cli/_no_train_modes.py:247-260, forgelm/compliance.py:2299, docs/qms/encryption_at_rest.md:180-191
  Recommended fix. Either implement a receipt-logging command (and audit the standalone export), or rewrite §4.6 to say receipt confirmation is operator-side and compliance.artifacts_exported marks production only. Also fix the 'ZIP' wording.

• CR-A02-113 (code, Low) — json-output.md reverted envelope shows a non-null final_model_path; the code emits null
  severity Low · reachability default · intent contract-violation · confidence likely · independently confirmed · reproduced with stubs · already tracked: PARTIALLY-FIXED C33-TRAIN-JSON-CONTRACT — docs/roadmap/phase-16-trust-surface-hardening.md:291,308 (S2 done); CHANGELOG.md:102-109
  Failure scenario. A consumer reads final_model_path on a reverted run and expects a path.
  Where. docs/usermanuals/en/reference/json-output.md:184, forgelm/trainer.py:1354-1373, docs/usermanuals/tr/reference/json-output.md:184
  Recommended fix. Fix the example and the mirror.

• CR-A02-114 (code, Low) — No audit event exists for config validation pass/fail
  severity Low · reachability default · intent contract-violation · confidence likely · independently confirmed · reproduced end to end
  Failure scenario. An auditor expects the documented validation pass/fail record and finds nothing for rejected or validated configs. The standard promises evidence that the code never produces.
  Where. docs/standards/logging-observability.md:113, forgelm/trainer.py:354-357, docs/reference/audit_event_catalog.md:30-110, docs/reference/audit_event_catalog.md
  Recommended fix. Either emit a config.validated / config.validation_failed event once training.output_dir is known, or narrow the standard's MUST list to what is emitted.

• CR-A02-115 (code, Low) — Non-finite baseline_loss silently disarms the loss gate
  severity Low · reachability default · intent intended-but-harmful · confidence confirmed · independently confirmed · reproduced (plumbing only) · already tracked: PARTIALLY-FIXED C31-NUMERIC-CONFIG — docs/roadmap/phase-16-trust-surface-hardening.md:289-292 (S2 done); CHANGELOG.md:9-20
  Failure scenario. Library caller: cfg.evaluation = EvaluationConfig.model_construct(auto_revert=True, baseline_loss=float('nan')), or a later assignment of NaN. The model's eval_loss regresses well past the pre-training loss. The gate returns True, emits evaluation.loss_gate_completed passed=True, does not revert, and the run succeeds with exit 0.
  Where. forgelm/trainer.py:826-836, forgelm/trainer.py:1314-1324 (confirmed), forgelm/trainer.py:868, forgelm/trainer.py:819-824
  Recommended fix. Fail closed on a non-finite configured baseline as max_acceptable_loss does, or treat it as None before _measure_baseline_loss runs so a real baseline is measured. Add a model_construct test with a NaN baseline.

• CR-A02-116 (code, Low) — Unreachable W&B/MLflow backend aborts training (exit 2), contrary to the documented warn-and-continue
  severity Low · reachability default · intent contract-violation · confidence likely · independently confirmed · reproduced end to end
  Failure scenario. An operator sets report_to: wandb and runs a nightly CI job. W&B is unreachable at job start. The doc promises a warning, but the run dies with exit 2 before any training step, after the wandb init timeout (90 s by default).
  Where. forgelm/trainer.py:519, forgelm/trainer.py:1228-1235, forgelm/cli/_training.py:149, docs/usermanuals/en/operations/experiment-tracking.md:142
  Recommended fix. Correct the EN/TR pitfall text: an unreachable backend fails the run at startup, and WANDB_MODE=offline or report_to: none avoids it. Alternatively, implement a deliberate opt-in fallback.

• CR-A02-117 (code, Low) — A completion that does not end with 'Answer:' still gets full correctness reward
  severity Low · reachability documented-opt-in · intent intended-and-acceptable · confidence likely · independently confirmed · reproduced end to end
  Failure scenario. The policy learns to append free text after the graded answer. That text, for example extra candidate answers on later lines that do not start with 'answer:', costs only the 0.8 format share, so correctness plus length can still beat a well-formatted wrong answer.
  Where. forgelm/trainer.py:219-235, forgelm/trainer.py:263-270, forgelm/grpo_rewards.py:120-129
  Recommended fix. Correct the docstring claim, or gate correctness on the same end-anchored match when strict format agreement is intended.

• CR-A02-118 (code, Low) — Reusing a ForgeConfig across ForgeTrainer runs carries run 1's measured baseline_loss (and OOM-halved batch/accumulation) into run 2, so the loss gate passes a model that regressed against its own ba…
  severity Low · reachability undocumented · intent intended-but-harmful · confidence confirmed · independently confirmed · reproduced with stubs · already tracked: KNOWN-OPEN TRUST-08 — docs/roadmap/phase-16-trust-surface-hardening.md:542-543 (S8 open)
  Failure scenario. Library caller: cfg = load_config(...) with evaluation.auto_revert: true, a validation split and no baseline_loss. Run 1 (ForgeTrainer(m1,t1,cfg,ds1).train()) measures baseline 2.0, trains to 1.5 and OOMs once (bs 4 to 2). The caller then runs ForgeTrainer(m2,t2,cfg,ds2).train() on a new base or dataset whose own pre-training loss is 1.0 and whose trained loss is 1.8, a +0.8 regression. Run 2 skips baseline measurement, compares 1.8 with run 1's 2.0, reports success=True and reverted=False, and…
  Where. forgelm/trainer.py:343, forgelm/trainer.py:1283, forgelm/trainer.py:1325, forgelm/trainer.py:1666-1667
  Recommended fix. Keep the measured baseline and the effective batch size on the trainer (for example self._effective_baseline_loss, self._effective_bs/_ga) instead of writing them into self.config. Or deep-copy the config in ForgeTrainer.init (self.config = config.model_copy(deep=True)) so the caller's object is never touched. Pin with a test that builds two trainers from one ForgeConfig and asserts that the…

• CR-A02-119 (code, Low) — The save_steps/eval_steps cadence rule is pinned only against a mock-built trainer, never against a real TrainingArguments/TRL Config, and not at --dry-run or CLI exit 1
  severity Low · reachability default · intent intended-and-acceptable · confidence likely · independently confirmed · reproduced end to end · already tracked: PARTIALLY-FIXED CORE-14 — docs/roadmap/phase-16-trust-surface-hardening.md:292 (S2 done); CHANGELOG.md:21-25
  Failure scenario. If transformers/TRL relax or tighten the rule (for example fractional steps, or a different rule when save<eval), ForgeLM's check silently diverges with all tests green. Likewise if the builder's output is changed so the real SFTConfig rejects it, no test sees it. Separately, nothing pins that eval=30/save=50 is rejected at config time (the gap behind M-A01-014), so the late failure is untested either way.
  Where. tests/test_trainer.py:1378-1441, tests/test_config.py:989-1010, forgelm/trainer.py:459-521, forgelm/config.py:1731-1748
  Recommended fix. Add a test that builds kwargs via ForgeTrainer._get_common_training_kwargs() and passes them to a real trl.SFTConfig (and one DPO/KTO-family config) for (30,60) expecting success and (30,50),(50,30),(100,50) expecting ConfigError from ForgeLM and ValueError from the bare upstream class. This pins agreement between the guard and the real class. Optionally add a config-level test that eval=30/save=…

• CR-A02-120 (code, Low) — A second ForgeTrainer.train() on the same instance is accepted without warning: it is logged as the same run, starts from the first run's weights (even ones the gate rejected), and overwrites that ru…
  severity Low · reachability undocumented · intent contract-violation · confidence confirmed · independently confirmed · reproduced end to end
  Failure scenario. An integrator retries trainer.train() after a reverted or failed run, or re-runs one instance after changing config fields. The second run silently fine-tunes the previous (possibly rejected) weights, reports success under the first run's run_id, overwrites the first run's checkpoints, final or staged model and compliance bundle, and leaves a manifest that claims a single training from the base model. An approver who reviewed run 1's metrics then approves run 2's different weights under the sam…
  Where. forgelm/trainer.py:354, forgelm/trainer.py:355-357, forgelm/trainer.py:365, forgelm/trainer.py:807-810
  Recommended fix. Make train() single-shot: set a flag on entry and raise a clear RuntimeError on a second call, telling the caller to build a new ForgeTrainer around a freshly loaded model. Alternatively, support re-entry explicitly: mint a new AuditLogger/run_id, emit pipeline.initialized, recompute _config_hash, restore a config snapshot, and refuse a model whose weights were already trained. Document the contr…

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