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

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-078 (code, Low) — Built-in GRPO rewards (and the classifier reward) assume string completions and crash on conversational message-list completions
  severity Low · reachability undocumented · intent contract-violation · confidence confirmed · independently confirmed · reproduced end to end · already tracked: KNOWN-OPEN CORE-02 — docs/roadmap/phase-16-trust-surface-hardening.md:390-392,402-404 (S4 open)
  Failure scenario. Once GRPO runs, an operator supplies chat-format prompts (TRL's common GRPO format) and training aborts at the first reward computation with exit 2.
  Where. forgelm/grpo_rewards.py:202 (len(completion) in length_shaping_reward; no crash, wrong value 1 message -> length 1), forgelm/grpo_rewards.py:202 (confirmed), forgelm/trainer.py:266 (ANSWER_EXTRACT_PATTERN.finditer(completion or '')), forgelm/grpo_rewards.py:182-189
  Recommended fix. Normalise each completion to text (take the last assistant content when given a list of message dicts) in one helper shared by all reward callables; or reject message-list prompts at data preparation with a clear error.

• CR-A02-079 (code, Low) — _ANSWER_END_PATTERN backtracks quadratically on single-line multi-marker completions despite its linear-time claim
  severity Low · reachability default · intent contract-violation · confidence likely · single reviewer · reproduced end to end
  Failure scenario. With long completions (tens of thousands of characters) that repeat 'answer:' on one line before a newline, format-reward scoring takes seconds per completion and slows every GRPO step.
  Where. forgelm/grpo_rewards.py:137-152, forgelm/grpo_rewards.py:189, forgelm/grpo_rewards.py:148-151, forgelm/grpo_rewards.py:172-184
  Recommended fix. Match only on the last line (rsplit('\n',1)[-1]) or use a fullmatch over that line; correct the comment and add the STD-069 linearity test.

• CR-A02-080 (code, Low) — The 'eval_steps larger than training dataset' warning compares optimizer steps with sample count
  severity Low · reachability default · intent intended-but-harmful · confidence likely · independently confirmed · not reproduced (code trace)
  Failure scenario. An operator relies on the absence of the warning and assumes early stopping is active, but no in-training evaluation ever happens.
  Where. forgelm/trainer.py:430-438
  Recommended fix. Compare eval_steps with the computed total optimizer steps, and warn regardless of auto_revert when early stopping is configured.

• CR-A02-081 (code, Low) — OOM recovery logs 'Effective batch size preserved' but changes it for non-power-of-two batch sizes
  severity Low · reachability documented-opt-in · intent contract-violation · confidence likely · independently confirmed · reproduced end to end
  Failure scenario. Batch 5 OOMs; the retry trains with effective batch 4 (20% smaller) under the unchanged learning rate while the log and docs claim nothing changed.
  Where. forgelm/trainer.py:1246-1262 (confirmed), docs/reference/configuration.md:107-108, tests/test_oom_recovery.py:370-420, forgelm/trainer.py:1243-1250
  Recommended fix. Choose new_bs as a divisor of current_bs (or compute ga = ceil(effective/new_bs)) and log the actual effective batch; record old/new effective batch in the training.oom_recovery event.

• CR-A02-082 (code, Low) — Approvals reference says the trainer never populates config_hash on human_approval.required, but it does
  severity Low · reachability default · intent contract-violation · confidence likely · single reviewer · not reproduced (code trace)
  Failure scenario. Operators told the field is always null do not use it as the reproducibility anchor it now is.
  Where. forgelm/trainer.py:1551, forgelm/cli/subcommands/_approvals.py:248, docs/reference/approvals_subcommand.md:73
  Recommended fix. Update the EN and TR approvals reference.

• CR-A02-083 (code, Low) — Errors raised in ForgeTrainer.init after pipeline.initialized leave the audit run without a terminal event
  severity Low · reachability default · intent contract-violation · confidence likely · independently confirmed · reproduced with stubs
  Failure scenario. An auditor reading audit_log.jsonl sees pipeline.initialized for run fg-... and no outcome. The log cannot tell a config refusal from a killed process.
  Where. forgelm/trainer.py:389-394, forgelm/trainer.py:388-394, forgelm/trainer.py:408-411, docs/reference/audit_event_catalog.md:30
  Recommended fix. Run the two validators before emitting pipeline.initialized, or wrap them so a failure emits pipeline.failed before re-raising. Correct the catalog wording.

• CR-A02-084 (code, Low) — Failures of the integrity manifest and deployer-instructions writers are warning-only and leave no audit marker
  severity Low · reachability default · intent intended-but-harmful · confidence confirmed · independently confirmed · not reproduced (code trace)
  Failure scenario. A disk-full during model_integrity.json leaves a run whose audit log shows success with no model.integrity_verified and no failure record. Separately, a permissions error on rmtree leaves a 'reverted' model in final_model/ that downstream tooling may pick up.
  Where. forgelm/trainer.py:2177-2189 (confirmed), forgelm/trainer.py:1789-1810, forgelm/trainer.py:2148-2155, forgelm/trainer.py:994-1007
  Recommended fix. Emit a *_failed audit event on each of these advisory failure paths, and emit a revert-incomplete event when rmtree fails.

• CR-A02-086 (code, Low) — Eval batch size is never set (HF default 8) and every evaluate() call sits outside OOM recovery
  severity Low · reachability default · intent intended-but-harmful · confidence confirmed · independently confirmed · reproduced (plumbing only)
  Failure scenario. A user with a 24 GB GPU sets per_device_train_batch_size=1 and oom_recovery=true to fit a 7B QLoRA at 4096 tokens. Training finishes, then trainer.evaluate() runs at bs=8 and OOMs. The run exits 2 after all the training compute, with no final_model written.
  Where. forgelm/trainer.py:500, forgelm/trainer.py:1293, forgelm/trainer.py:1588, forgelm/trainer.py:1612
  Recommended fix. Set per_device_eval_batch_size = min(8, per_device_train_batch_size) (or the train value) in _get_common_training_kwargs, and update it on each OOM rebuild. Save the final model before the post-train evaluate, or wrap the evaluate calls in the same halving loop. Expected gain: avoids losing a completed run to an eval-time OOM; lower peak VRAM for eval by up to 8x at bs=1.

• CR-A02-087 (code, Low) — OOM retry restarts at step 0 and reuses the same EarlyStoppingCallback instance, carrying its patience counter into the new run
  severity Low · reachability documented-opt-in · intent intended-but-harmful · confidence likely · independently confirmed · not reproduced (code trace)
  Failure scenario. OOM at 80% of an epoch after 2 non-improving evals. The retry starts at step 0 with counter=2, and the first non-improving eval stops the new run at patience 3.
  Where. forgelm/trainer.py:1231 (retry re-enters train() with the original resume_from_checkpoint), forgelm/trainer.py:1671-1677 (single EarlyStoppingCallback instance created and stashed), forgelm/trainer.py:1231, forgelm/trainer.py:1270 (_build_trainer(callbacks) on retry)
  Recommended fix. Rebuild the callbacks (a fresh EarlyStoppingCallback) on each retry. Better, resume from the latest on-disk checkpoint-* so completed steps are not redone, which the code already lists as a future item. Expected gain: avoids repeating up to the whole pre-OOM prefix of the run.

• CR-A02-088 (code, Low) — Two tests assert a reconstruction of the code instead of the code (tautological), and one claims to drive the call site but evals scraped source
  severity Low · reachability default · intent contract-violation · confidence likely · single reviewer · reproduced end to end
  Failure scenario. The staging directory naming or the reverted derivation regresses (an earlier fix for reverted=True hardcoding is the history the test was written for), and the guard test still passes because it checks a copy of the logic.
  Where. tests/test_human_approval_gate.py:147-163, tests/test_human_approval_gate.py:127-145, tests/test_trainer.py:1443-1538, tests/test_trainer_grpo_config.py:165-205
  Recommended fix. Replace both with tests that call the real _run_training_pipeline (see the previous finding) and assert on the returned TrainResult and on-disk state. Delete the source-scraping helper.

• CR-A02-089 (code, Low) — Opt-in kwarg wiring for GaLore, NEFTune, DeepSpeed and FSDP is never asserted; tests stop at the config schema
  severity Low · reachability documented-opt-in · intent contract-violation · confidence confirmed · independently confirmed · reproduced end to end
  Failure scenario. A rename in TrainingArguments, or a typo in the optim_args string, makes GaLore or ZeRO silently inactive or crash at first step; no test notices.
  Where. forgelm/trainer.py:544-561, forgelm/trainer.py:591-615 (_apply_distributed_config; deepspeed kwarg 595-599, fsdp offload 605-606, unknown-strategy warning 615), forgelm/trainer.py:617-670 (_resolve_deepspeed_config CWD fallback 652-654), tests/test_galore.py
  Recommended fix. Call _get_common_training_kwargs on configs with each feature enabled and assert the exact kwargs. Then feed them into the real TrainingArguments(output_dir=tmp_path, use_cpu=True) to validate the shape (the optim string and fsdp strings are the risky ones).

• CR-A02-092 (code, Low) — Experiment tracking via report_to and the [tracking] / [tracking-mlflow] extras has no test at all
  severity Low · reachability default · intent contract-violation · confidence likely · independently confirmed · reproduced end to end
  Failure scenario. A refactor drops or hard-codes the report_to key. Every run silently stops logging to W&B or MLflow while all tests stay green.
  Where. forgelm/trainer.py:519, forgelm/config.py:604-607, pyproject.toml:87-103, tests/test_integration.py:66
  Recommended fix. Add parametrized trainer-config tests asserting report_to is forwarded for each value, and a pyproject extras test (tracking/tracking-mlflow exist and pin wandb/mlflow). Cover the missing-package error path.

• CR-A02-093 (code, Low) — No artefact carries a schema/format version; readers branch on key presence or forgelm_version, and several writers are non-atomic or change file permissions
  severity Low · reachability default · intent intended-but-harmful · confidence likely · single reviewer · not reproduced (code trace)
  Failure scenario. A format change in a later release cannot be detected by an older verifier or auditor tool; an interrupted write leaves a truncated governance or integrity file; a 0600 audit report is unreadable by a separate CI reader user.
  Where. forgelm/trainer.py:2097, forgelm/trainer.py:2165, forgelm/benchmark.py:113-135, forgelm/data_audit/_orchestrator.py:143-190
  Recommended fix. Add a schema_version integer to each persisted artefact and have readers reject unknown majors; centralise one atomic_write helper (temp + fsync + replace + umask-based mode) and use it for every artefact.

• CR-A02-094 (code, Low) — Manifest records no library versions or hardware-derived training knobs
  severity Low · reachability default · intent intended-but-harmful · confidence likely · single reviewer · reproduced end to end
  Failure scenario. The same YAML on CUDA bf16 and CPU fp32 gives different weights and the manifest shows neither.
  Where. forgelm/trainer.py:444-520, forgelm/compliance.py:1500-1545
  Recommended fix. Add an environment block (library versions, device kind, dtype, optimizer, seed).

• CR-A02-095 (code, Low) — OOM recovery mutates the live config, so config_hash and manifest describe the post-recovery run, while weights carry over
  severity Low · reachability documented-opt-in · intent intended-but-harmful · confidence likely · single reviewer · not reproduced (code trace)
  Failure scenario. After an OOM retry, a verifier re-hashing the YAML sees a config_hash mismatch for an untampered run.
  Where. forgelm/trainer.py:1264-1265, forgelm/compliance.py:1516
  Recommended fix. Keep the original config for hashing/manifest and record effective values separately, with a 'recovered_from_oom' flag.

• CR-A02-096 (code, Low) — The validation split that selects the checkpoint (best-model and early stopping) is reused unchanged as the loss gate's evidence, and the comparison has no noise margin
  severity Low · reachability documented-opt-in · intent intended-and-acceptable · confidence likely · independently confirmed · not reproduced (code trace)
  Failure scenario. A run with many eval checkpoints on a small validation split passes the baseline check because of selection noise, not real improvement.
  Where. forgelm/trainer.py:459, forgelm/trainer.py:511-513, forgelm/trainer.py:1588, forgelm/trainer.py:1671-1673
  Recommended fix. Document the bias, or gate on a held-out split (or the pre-selection final checkpoint), and allow a configurable tolerance on the baseline comparison.

• CR-A02-097 (code, Low) — Trainer writes raw exception strings into the append-only audit log (pipeline.failed, compliance.*_failed), while the same string is masked for the webhook
  severity Low · reachability default · intent intended-but-harmful · confidence likely · independently confirmed · not reproduced (code trace)
  Failure scenario. A collator or TRL exception that quotes the offending example (or a URL with a token) is copied permanently into audit_log.jsonl. Under Art. 17 it cannot be erased without breaking the hash chain.
  Where. forgelm/trainer.py:1706, forgelm/trainer.py:2127, forgelm/trainer.py:2155, forgelm/cli/subcommands/_purge.py:102-130
  Recommended fix. Route every audit 'error'/'reason' string from exceptions through _sanitise_audit_error_message (mask_secrets + mask_pii + length bound), as purge and reverse-pii already do.

• CR-A02-098 (code, Low) — Deployer instructions and the integrity manifest record the staging path as the model location after human approval
  severity Low · reachability documented-opt-in · intent intended-but-harmful · confidence likely · single reviewer · not reproduced (code trace)
  Failure scenario. After approval, a deployer reading final_model/deployer_instructions.md sees 'Model Location: .../final_model.staging.fg-xxxx', a path that was removed by the rename.
  Where. forgelm/trainer.py:1611, forgelm/trainer.py:1652, forgelm/compliance.py:1697, forgelm/cli/subcommands/_approve.py:589
  Recommended fix. Record the intended final path (train_result.final_model_path before staging) or a relative location, or regenerate both files on approve.

• CR-A02-099 (code, Low) — Wandb and mlflow trackers give HF's install hint, not the forgelm extra
  severity Low · reachability documented-opt-in · intent intended-but-harmful · confidence likely · single reviewer · reproduced (plumbing only)
  Failure scenario. The operator sets report_to: wandb without the extra and the run dies at trainer construction (exit 2).
  Where. forgelm/trainer.py:519, forgelm/config.py:604
  Recommended fix. Add a config-time check with the extras hint.

• CR-A02-100 (code, Low) — Code does not match docs/reference/verify_integrity_subcommand.md: 'The compliance export writes a SHA-256 hash of every file in the model directory'; 'The manifest itself (model_integrity.json) is e…
  severity Low · reachability default · intent contract-violation · confidence confirmed · independently confirmed
  Failure scenario. The document states: 'The compliance export writes a SHA-256 hash of every file in the model directory'; 'The manifest itself (model_integrity.json) is excluded from this walk'. Observed in the code: model_integrity.json is written by the training run's finalize step, not by --compliance-export (which writes 4 files and no model_integrity.json). The writer does not exclude a pre-existing model_integrity.json (F2), and the verifier excludes the manifest only from the added-file walk, not from…
  Where. forgelm/trainer.py:1503-1505, forgelm/compliance.py:785-787, forgelm/compliance.py:760-820, forgelm/trainer.py:2157-2168
  Recommended fix. Say the manifest is written at the end of every training run; once F2 is fixed, state that the exclusion applies when the manifest is generated and when it is verified

• ↳ same root cause: DOC-D1B04-067 (docs, Low) — docs/reference/verify_integrity_subcommand.md: misleading — 'The compliance export writes a SHA-256 hash of every file in the model directory'; 'The manifest itself (model_integrity.json) is excluded…
  severity Low
  Where. docs/reference/verify_integrity_subcommand.md:6,45; forgelm verify-integrity --help · type: misleading
  The document says. 'The compliance export writes a SHA-256 hash of every file in the model directory'; 'The manifest itself (model_integrity.json) is excluded from this walk'
  What the code does. model_integrity.json is written by the training run's finalize step, not by --compliance-export (which writes 4 files and no model_integrity.json). The writer does not exclude a pre-existing model_integrity.json (F2), and the verifier excludes the manifest only from the added-file walk, not from recorded entries.
  Code references. forgelm/trainer.py:1496-1506, forgelm/compliance.py:787
  Proposed correction. Say the manifest is written at the end of every training run; once F2 is fixed, state that the exclusion applies when the manifest is generated and when it is verified
  Turkish mirror. the Turkish page has the same error; fix both in the same PR

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