Skip to content

[Wave 3] Medium — Module sweep: Model loading, dataset preparation & VRAM fit-check (part 1/2) #559

Open

Description

@cemililik

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

Findings

  • CR-A03-010 (code, Medium) — Auto-split always shuffles regardless of data.shuffle and logs the wrong validation size
    severity Medium · reachability default · intent contract-violation · confidence confirmed · independently confirmed · reproduced end to end
    Failure scenario. An operator reading the log believes the validation set has 1 row when it has 2. A cosmetic mismatch with the Article 10 governance report.
    Where. forgelm/data.py:580 (unconditional train_test_split shuffle), forgelm/data.py:549-581 (confirmed), forgelm/data.py:712 (call site in prepare_dataset), forgelm/data.py:677-680 (_format_sft_dataset shuffle branch)
    Recommended fix. Log len(split_dataset['test']) after splitting.

  • CR-A03-011 (code, Medium) — Unsloth backend silently ignores rope_scaling, sliding_window, moe, multimodal and bnb_4bit_*; PiSSA is rejected only after the load
    severity Medium · reachability documented-opt-in · intent contract-violation · confidence confirmed · independently confirmed · reproduced with stubs
    Failure scenario. An operator sets backend unsloth with bnb_4bit_quant_type fp4 and moe.experts_to_train. The run trains NF4 with all experts, and the compliance artefacts that echo the config overstate what happened.
    Where. forgelm/model.py:339-351, forgelm/model.py:342-351 (confirmed), forgelm/model.py:560-561, docs/usermanuals/en/training/distributed.md:59
    Recommended fix. Validate at config time: reject or warn on each field the unsloth backend ignores, and move the PiSSA rejection into ForgeConfig validation. Remove or implement the fallback claim in the docs.

  • CR-A03-014 (code, Medium) — data.mix_ratio is a cap relative to the largest dataset, not a realised proportion
    severity Medium · reachability documented-opt-in · intent contract-violation · confidence confirmed · independently confirmed · reproduced end to end · already tracked: KNOWN-OPEN CORE-11 — docs/roadmap/phase-16-trust-surface-hardening.md:588-589,605-606 (S10 open)
    Failure scenario. Operator expects a 50/50 blend from two unequal corpora and trains on an 83/17 one.
    Where. forgelm/data.py:496-509 (confirmed), docs/guides/troubleshooting.md:285, docs/usermanuals/en/concepts/data-formats.md:138, forgelm/config.py:813-816 (disputed)
    Recommended fix. Find the largest total N such that n_i = N*w_i <= len(ds_i) for all i (N = min_i len_i/w_i). Optionally allow upsampling with replacement. Log realised counts and shares, and warn when the declared ratio cannot be met.

  • CR-A03-015 (code, Medium) — fit-check parameter count ignores MoE experts, nested VLM text_config, explicit head_dim and untied/bf16 embeddings, which gives a false FITS for Mixtral-class models
    severity Medium · reachability default · intent intended-but-harmful · confidence confirmed · independently confirmed · reproduced with stubs
    Where. forgelm/fit_check.py:93-98, forgelm/fit_check.py:124-133, forgelm/fit_check.py:147-180, forgelm/fit_check.py:160
    Recommended fix. Read num_local_experts/num_experts (and shared experts), get_text_config(), head_dim, and tie_word_embeddings. Price embeddings and lm_head at the compute dtype under 4-bit. Add a note whenever the architecture comes from hints or defaults. As a cheaper and more accurate alternative, count parameters by instantiating on the meta device.

  • CR-A03-017 (code, Medium) — fit-check documentation describes components, accuracy and multi-GPU sharding that the estimator does not compute; trainer_type and distributed setups are not modelled
    severity Medium · reachability default · intent contract-violation · confidence confirmed · independently confirmed · reproduced with stubs
    Where. forgelm/fit_check.py:278-311, forgelm/fit_check.py:314-334, docs/usermanuals/en/operations/vram-fit-check.md:13-101
    Recommended fix. Rewrite the manual page to match the actual output (breakdown keys, meaning of UNKNOWN, no multi-GPU support), or implement the documented terms: trainer_type multipliers, logits, ZeRO world-size division.

  • CR-A03-019 (code, Medium) — data.clean_text collapses all newlines in text and User/Assistant rows but is skipped for messages and non-SFT rows
    severity Medium · reachability default · intent contract-violation · confidence confirmed · independently confirmed · reproduced end to end
    Failure scenario. A messages-format or DPO corpus containing NUL/ESC control characters is trained uncleaned while the Annex IV data_provenance block asserts clean_text: true. An operator who sets shuffle: false to keep a time-ordered tail as validation gets a random split.
    Where. forgelm/data.py:51-87 (confirmed), forgelm/data.py:354 (confirmed), forgelm/data.py:486-487, forgelm/config.py:818-820
    Recommended fix. Collapse only runs of horizontal whitespace and runs of blank lines, keep newlines and indentation, and apply the same cleaning to message contents (or document the exemption).

  • CR-A03-022 (code, Medium) — A missing relative local dataset path is classified as a Hub dataset id and resolved on the Hub
    severity Medium · reachability default · intent intended-but-harmful · confidence confirmed · independently confirmed · reproduced end to end
    Failure scenario. Operator mistypes a path; gets a Hub-flavoured error after waiting for the model load.
    Where. forgelm/data.py:187-220 (confirmed), forgelm/data.py:312-337 (confirmed), forgelm/data.py:698, forgelm/compliance.py:1296-1298
    Recommended fix. Treat a string ending in a supported data extension (.jsonl/.json/.csv/.parquet) that is not an existing file as a local-path error with an actionable message, before any Hub call.

  • CR-A03-023 (code, Medium) — prepare_dataset orchestration, trainer-type validation, passthroughs, multimodal and SFT formatting are never executed by any test
    severity Medium · reachability default · intent contract-violation · confidence confirmed · independently confirmed · reproduced end to end
    Failure scenario. Each of these mutants survived the targeted set. The first five cannot be caught at all, because the lines are unexecuted. D3 deletes the _validate_trainer_columns call, so a DPO config on an SFT-shaped dataset trains instead of raising KeyError. D4 removes the passthrough, so DPO, KTO and GRPO rows get SFT text-formatted and lose chosen/rejected/prompt columns. D7 passes mix_ratio=None, so the configured mixture is ignored. D7b ignores extra_datasets. D16 forces add_eos=True. D8 never appends…
    Where. forgelm/data.py:483-490, forgelm/data.py:625-666, forgelm/data.py:675-688, forgelm/data.py:699-733
    Recommended fix. Add tests that run the real prepare_dataset on tmp_path JSONL files and a SimpleNamespace tokenizer with eos_token. Cover sft, dpo, simpo, orpo, kto and grpo plus a wrong-schema case that expects KeyError, a multimodal config with and without the image column, extra_datasets with mix_ratio, add_eos and clean_text on and off, and split shuffling. Assert the returned columns, split sizes and text.…

  • CR-A03-024 (code, Medium) — MoE expert quantization and recast have no behavioural assertions; two MoE tests are vacuous
    severity Medium · reachability documented-opt-in · intent contract-violation · confidence confirmed · independently confirmed · reproduced end to end · already tracked: KNOWN-OPEN OPS-21 — docs/roadmap/phase-16-trust-surface-hardening.md:723-726,738-740 (S14 open)
    Failure scenario. M7 (quantize_experts ignored), M8 (recast becomes a no-op) and M9 (_is_frozen_expert_weight inverted, so trainable expert weights get recast) all survive the whole suite. Recasting trainable weights, or doing nothing while the config promises VRAM savings, would go unnoticed.
    Where. forgelm/model.py:512-513, forgelm/model.py:617-641, tests/test_moe_functions.py:42-81, tests/test_moe_functions.py:82-102
    Recommended fix. Use real torch.nn.Linear modules. Assert that frozen fp32 expert weights become the target dtype, that requires_grad=True weights are unchanged, that the counts match the log, and that a failed recast is counted. Test _apply_moe_post_peft with quantize_experts=True and with 'all'. Replace the frozen_count > 0 checks with exact sets of frozen parameter names.

  • CR-A03-025 (code, Medium) — Site wizard max_length hint claims RoPE scaling kicks in above 4096; it is applied only when configured
    severity Medium · reachability default · intent contract-violation · confidence confirmed · independently confirmed · not reproduced (code trace)
    Failure scenario. A user sets max_length 16384 on a 4K-context model, expecting automatic RoPE scaling. None is applied.
    Where. site/js/translations.js:4668, forgelm/model.py:463-466 (confirmed), site/js/wizard.js:2042-2047, forgelm/model.py:463
    Recommended fix. Correct the hint in 6 locales, or expose rope_scaling.

  • CR-A03-027 (code, Medium) — LoRA adapter initialisation is unseeded: two identical runs give different adapters; no seed knob; seed not recorded; docs promise byte-reproducibility
    severity Medium · reachability default · intent contract-violation · confidence confirmed · independently confirmed · reproduced end to end
    Failure scenario. An operator or auditor re-runs a pinned config (model.revision SHA, same dataset) to reproduce a model, or re-trains after purge. The result is a different adapter, so loss, eval gates and safety verdicts can differ run to run with no config diff. The manifest cannot say what seed was used.
    Where. forgelm/model.py:609, forgelm/model.py:573-589, forgelm/trainer.py:444-520, forgelm/config.py:753
    Recommended fix. Add training.seed (and data_seed) to the schema, default 42. Call transformers.set_seed(seed) at the top of get_model_and_tokenizer (before get_peft_model, and before the unsloth peft call) and pass seed/data_seed to the TRL config. Record the seed and full_determinism in training_parameters. Soften the byte-for-byte wording until then.

  • CR-A03-029 (code, Medium) — No leakage control between the data the model trains on and the sets the gates evaluate on: the auto-split keeps exact duplicates on both sides, and probes and judge sets are never compared with the…
    severity Medium · reachability default · intent intended-but-harmful · confidence confirmed · independently confirmed · reproduced end to end
    Failure scenario. An overfit or memorising fine-tune shows a falsely low eval_loss and clears max_acceptable_loss and the baseline check. Separately, a dataset publisher includes the bundled probes with refusals, and the safety gate passes a model that complies with novel harmful requests.
    Where. forgelm/data.py:549-581, forgelm/data.py:512-546, forgelm/data.py:691-733, forgelm/trainer.py:1272-1327
    Recommended fix. Deduplicate, or at least count and warn on exact overlap, before auto-splitting. Run the existing simhash/minhash cross-split check on the in-memory train/validation pair and on train versus the probe and judge prompt sets. Record the overlap counts in the audit event.

  • CR-A03-035 (code, Medium) — Usermanual DPO page tells operators to mix SFT rows via data.extra_datasets; doing so crashes the DPO run (exit 2)
    severity Medium · reachability default · intent contract-violation · confidence confirmed · independently confirmed · reproduced end to end
    Where. docs/usermanuals/en/training/dpo.md:220, forgelm/data.py:513-546, forgelm/data.py:701-721, forgelm/data.py:584-599
    Recommended fix. Remove the advice from the doc, or have _merge_extra_datasets reject extra datasets whose format does not match the trainer type at config or load time (exit 1).

  • CR-A03-039 (code, Medium) — An optional System column crashes the run when only some rows carry it
    severity Medium · reachability default · intent contract-violation · confidence confirmed · independently confirmed · reproduced end to end
    Where. forgelm/data.py:416-431, forgelm/data.py:73-83, docs/reference/data_preparation.md:28, forgelm/data.py:454
    Recommended fix. Treat a None System cell as absent (blank), or document that System must be present on every row once any row has it.

  • CR-A03-040 (code, Medium) — model.offline does not stop the dataset load itself from reaching the Hub for library consumers
    severity Medium · reachability default · intent contract-violation · confidence confirmed · independently confirmed · reproduced end to end
    Failure scenario. An air-gapped notebook or service builds ForgeConfig(model={'offline': True, ...}, data={'dataset_name_or_path': 'org/corpus'}) and calls prepare_dataset or ForgeTrainer. datasets.load_dataset resolves huggingface.co and sends requests that disclose the repo id, and the run waits on retries, even though the config says offline.
    Where. forgelm/data.py:306-308, forgelm/data.py:697-698, forgelm/data.py:535, forgelm/data.py:323-324
    Recommended fix. Make the load honour the offline argument, or narrow compliance_summary.md:144-148 to say that only the provenance lookups are argument-driven and the dataset load still needs the env vars.

  • CR-A03-041 (code, Medium) — User manual documents an Unsloth fallback to the standard backend that does not exist
    severity Medium · reachability documented-opt-in · intent contract-violation · confidence confirmed · independently confirmed · not reproduced (code trace)
    Failure scenario. An operator relies on the documented fallback for an architecture Unsloth does not support. The run aborts instead of continuing on transformers.
    Where. docs/usermanuals/en/training/distributed.md:59, docs/usermanuals/tr/training/distributed.md:59, forgelm/model.py:290-295, forgelm/model.py:560-561 (confirmed)
    Recommended fix. Remove the fallback sentence, or implement the fallback.

  • CR-A03-042 (code, Medium) — KTO loader accepts integer labels the guide says are rejected; string labels crash at train time
    severity Medium · reachability default · intent contract-violation · confidence confirmed · independently confirmed · reproduced end to end
    Failure scenario. An operator relying on the promised rejection uses 1/0 labels and gets no error. The run appears equivalent here, but the documented safety net does not exist. A 'true'/'false' string-labelled dataset loads, then the run dies mid-training with TypeError.
    Where. forgelm/data.py:600-607, forgelm/data.py:720, docs/usermanuals/en/training/kto.md:85, docs/usermanuals/tr/training/kto.md:85
    Recommended fix. Validate that label is bool in _validate_trainer_columns (check the first N rows or the dataset features), or correct kto.md:85.

  • CR-A03-045 (code, Medium) — Benchmark gate cannot run in multimodal (VLM) mode: ForgeLM passes an AutoProcessor where lm-eval HFLM asserts a tokenizer class, and the gate fails with an empty reason
    severity Medium · reachability documented-opt-in · intent contract-violation · confidence confirmed · independently confirmed · reproduced (plumbing only)
    Failure scenario. A VLM fine-tune with benchmark tasks and auto_revert=true trains to completion. HFLM.init raises a bare AssertionError, the gate returns passed=False with an empty reason after the colon, and final_model is deleted (exit 3).
    Where. forgelm/model.py:381-388, forgelm/trainer.py:1778-1787, forgelm/benchmark.py:178-181, forgelm/model.py:573 (get_model_and_tokenizer passes it on)
    Recommended fix. Reject benchmark+multimodal in config validation, or pass the inner text tokenizer (getattr(tokenizer, 'tokenizer', tokenizer)) and document that lm-eval text tasks score only the language model. Put type(e).name in failure_reason so empty-message exceptions stay diagnosable.

  • CR-A03-047 (code, Medium) — Nonexistent model.revision: --dry-run passes, local-dir pins are accepted silently with a nonexistent ref recorded as revision_pinned, and the cache-miss error is misleading and does not name the rev…
    severity Medium · reachability default · intent contract-violation · confidence confirmed · independently confirmed · reproduced end to end
    Failure scenario. A deployer pins a SHA with a typo and gets a green --dry-run. At run time the failure is exit 2 (retry class) with a message blaming the network on an offline host. Alternatively, the name_or_path is a local dir and the pin typo is accepted, and the compliance bundle states a pin that was never applied.
    Where. forgelm/model.py:97-165, forgelm/model.py:372-394, forgelm/compliance.py:1296-1298, forgelm/config.py:304-323
    Recommended fix. Catch the OSError/RepositoryNotFoundError/RevisionNotFoundError from the pinned loads, re-raise ConfigError naming model.name_or_path and model.revision (exit 1), and when offline say 'not in the local cache for revision X'. Either fail closed or warn loudly for a revision on a local path at load time, as config.py:305-323 promises, and do not write revision_pinned for a local path. Fix the --dry…

  • CR-A03-048 (code, Medium) — SFT loss is computed on every token, system prompt and user turns included. Nothing can switch this off, and no doc says so
    severity Medium · reachability default · intent intended-but-harmful · confidence confirmed · independently confirmed · reproduced end to end
    Failure scenario. A support-bot dataset puts the same 200-token system prompt and long user tickets in front of short assistant replies. ForgeLM spends most of the SFT gradient on reproducing the system prompt and the user text. The tuned model learns to emit user-style text and system-prompt boilerplate, and the training loss is dominated by tokens it will never be asked to produce. The operator cannot see this from the docs and cannot change it in the config.
    Where. forgelm/data.py:387, forgelm/data.py:414-431 (line 428), forgelm/data.py:729-734, forgelm/trainer.py:691-720
    Recommended fix. State in sft.md (EN and TR) and data-formats.md that SFT loss covers all tokens, system and user turns included. Alternatively, keep conversational and prompt/completion data in a shape TRL understands and expose a config field that maps to completion_only_loss / assistant_only_loss. Add a test that asserts which positions carry labels of -100.

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

No one assigned

    Labels

    area: model-dataCode area: model-databacklog: knownAlready tracked in docs/roadmapbugSomething isn't workingsecuritySecurity-relevantseverity: mediumReal defect with limited reach or a workaroundsource: review-2026-10Found by the October 2026 code and documentation reviewwave: 3Roadmap wave 3: medium severity, module sweeps

    Type

    No type

    Projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions