Repository navigation
[Wave 3] Medium — Module sweep: Post-training: inference, chat, GGUF export, deploy, merging (part 1/2) #567
Description
Wave 3 · 22 finding(s) in 20 fix package(s). Fix these whenever a pull request touches this module ("clean the module you touch").
Findings
-
CR-A13-008 (code, Medium) — Forgelm chat --temperature 0 is accepted but every turn fails; REPL refuses /temperature 0
severity Medium · reachabilitydefault· intentcontract-violation· confidence confirmed · independently confirmed · reproduced (plumbing only) · already tracked: KNOWN-OPEN CLI-17 — docs/roadmap/phase-16-trust-surface-hardening.md:480,500-501 (S7 open; unit id inferred)
Failure scenario. An operator wanting greedy, reproducible probing gets an unusable session.
Where.forgelm/chat.py:201-210(REPL rejects 0),forgelm/chat.py:273-289(soft-fail per turn),forgelm/inference.py:247-248, 285-286 (do_sample=True default, temperature passed),forgelm/inference.py:306-307
Recommended fix. Map temperature == 0 to do_sample=False in inference.generate*, allow it in /temperature, or narrow the parser range and docs. -
↳ same root cause: DOC-D3B08-061 (docs, Low) — docs/usermanuals/en/deployment/chat.md: wrong detail — /temperature takes 0.0 to 2.0; /temperature 0 is near-deterministic
severity Low
Where.docs/usermanuals/en/deployment/chat.md:34,125 · type: wrong detail
The document says. /temperature takes 0.0 to 2.0; /temperature 0 is near-deterministic
What the code does. REPL rejects 0 (0.0 < t <= 2.0); CLI --temperature 0 errors each turn.
Code references.forgelm/chat.py:204,forgelm/inference.py:248
Proposed correction. Fix code (F4) or document (0.0, 2.0].
Turkish mirror. the Turkish page has the same error; fix both in the same PR -
↳ same root cause: DOC-D3B08-062 (docs, Low) — docs/usermanuals/en/deployment/chat.md: false claim — /temperature takes 0.0 to 2.0; pitfall describes /temperature 0 as near-deterministic
severity Low
Where.docs/usermanuals/en/deployment/chat.md:34,125 · type: false claim
The document says. /temperature takes 0.0 to 2.0; pitfall describes /temperature 0 as near-deterministic
What the code does. The REPL rejects 0 (range 0.0 < t <= 2.0); the CLI --temperature 0 is accepted but each generation fails with a transformers error.
Code references.forgelm/chat.py:204,forgelm/inference.py:248-249
Proposed correction. Fix the code to map 0 to greedy, or document 0 < t <= 2.0 and delete the /temperature 0 pitfall; also note there is no seed: YAML for chat.
Turkish mirror. the Turkish page has the same error; fix both in the same PR -
CR-A13-009 (code, Medium) — Export --adapter writes into, then rmtree's, a fixed '_merged_for_export' sibling, and leaks it on some failure paths
severity Medium · reachabilitydefault· intentintended-but-harmful· confidence confirmed · independently confirmed · reproduced end to end · already tracked: KNOWN-OPEN C33-EXPORT-TEMP-OWNERSHIP — docs/roadmap/phase-16-trust-surface-hardening.md:450-459 (S6 open)
Failure scenario. An operator keeps a merged model under the same naming convention and loses it.
Where.forgelm/export.py:446-450,forgelm/export.py:363-369,forgelm/export.py:449-450,forgelm/export.py:155-185
Recommended fix. Use tempfile.mkdtemp or refuse when the path exists. -
CR-A13-011 (code, Medium) — load_model on MPS defaults to bf16 and silently returns empty generations; chat and safety eval do not notice
severity Medium · reachabilitydefault· intentintended-but-harmful· confidence confirmed · independently confirmed · reproduced end to end
Failure scenario. On a Mac, chat appears blank on every turn and a safety run passes vacuously.
Where.forgelm/inference.py:106(AutoModelForCausalLM.from_pretrained(path, **model_kwargs)),forgelm/chat.py:252-300(_generate_and_print prints and appends an empty assistant turn at 299-300),forgelm/safety/_orchestrator.py:239(_generate_safety_responses call),forgelm/inference.py:41-106(load_model: device_map='auto', no dtype argument)
Recommended fix. Warn when a non-trivial prompt yields zero new tokens, and expose a dtype/device option on chat/safety-eval. -
CR-A13-012 (code, Medium) — Forgelm export does not guard export_model: exceptions outside the converter call escape as tracebacks with exit 1 and no JSON envelope
severity Medium · reachabilitydefault· intentcontract-violation· confidence confirmed · independently confirmed · reproduced end to end · already tracked: KNOWN-OPEN C33-CLI-ERROR-ENVELOPE — docs/roadmap/phase-16-trust-surface-hardening.md:507-508 (S7 open)
Failure scenario. A CI step parses export's JSON stdout. A bad output path or a disk-full during hashing yields empty stdout, so the jq gate errors on parse rather than reading a structured failure.
Where.forgelm/cli/subcommands/_export.py:16-23(confirmed),forgelm/export.py:440-441(confirmed),forgelm/export.py:461-462(confirmed),forgelm/cli/_dispatch.py:113-121(confirmed)
Recommended fix. Wrap export_model in the dispatcher with an OSError catch that emits the envelope and exits 2 (or 1 for path errors), or convert these OSErrors to ExportResult inside export_model. -
CR-A13-013 (code, Medium) — Export writes exported_artifacts, which verify-integrity never reads; a GGUF exported into the model dir turns verify-integrity into exit 6
severity Medium · reachabilitydefault· intentintended-but-harmful· confidence confirmed · independently confirmed · reproduced end to end · already tracked: KNOWN-OPEN CLI-03 — docs/roadmap/phase-16-trust-surface-hardening.md:426-429 (S5 open)
Failure scenario. A release gate runs export then verify-integrity on the same directory and pages a false tamper event, or the operator disables the checks.
Where.forgelm/export.py:207-239,forgelm/export.py:493-494(confirmed),forgelm/cli/subcommands/_export.py:22,forgelm/verify/_model_integrity.py:111(confirmed)
Recommended fix. Have verify_integrity treat exported_artifacts as recorded (hash-check them) or have export not write into artifacts-tracked dirs; add a writer-then-verifier test and a --no-integrity-update test. -
CR-A13-014 (code, Medium) — TIES sign election counts sign votes instead of using sign of the summed deltas; two-adapter conflicts always resolve positive
severity Medium · reachabilitydefault· intentintended-but-harmful· confidence confirmed · independently confirmed · reproduced end to end
Failure scenario. Default TIES merge of two adapters: wherever they disagree, a tiny positive delta from a weight-0.1 adapter overrides a large negative delta from a weight-0.9 adapter. The merged model drifts positive on every conflicting weight, and the run exits 0.
Where.forgelm/merging.py:408-427(confirmed),forgelm/merging.py:262-264,forgelm/config.py:186-188,docs/usermanuals/en/deployment/model-merging.md:26
Recommended fix. Use magnitude-weighted sign election (the paper's method), weighting by the adapter weights, and replace the test that pins 'ties go to +1'. -
CR-A13-015 (code, Medium) — TIES/DARE merge keeps N+4 full-model copies, including full-size zero deltas for untouched tensors
severity Medium · reachabilitydefault· intentintended-but-harmful· confidence confirmed · independently confirmed · reproduced end to end
Failure scenario. A default TIES merge of two 7B adapters is OOM-killed on a 64 GB host, although only the attention projections differ from the base.
Where.forgelm/merging.py:306,forgelm/merging.py:306-307(confirmed),forgelm/merging.py:338-365,forgelm/merging.py:297-308
Recommended fix. Store deltas only for keys that changed (torch.count_nonzero), and call state_dict() once per adapter. -
CR-A13-016 (code, Medium) — Export --adapter writes the integrity record into the base model's manifest and never into the run's own; failures are swallowed at DEBUG and the write is not atomic
severity Medium · reachabilitydefault· intentcontract-violation· confidence confirmed · independently confirmed · reproduced end to end
Failure scenario. An auditor checks the training run's model_integrity.json for the shipped GGUF hash and finds nothing, or finds the hash recorded in a different model's manifest. A disk-full error during the rewrite leaves the run manifest truncated, and the only trace is a DEBUG log line.
Where.forgelm/export.py:207-239,forgelm/export.py:493-494,forgelm/verify/_model_integrity.py:103-111,notebooks/post_training_workflow.ipynb:290
Recommended fix. Write to the adapter dir's manifest when --adapter is given, or to a manifest the operator names. Use tmp + fsync + os.replace, as compliance.py:462-472 already does. Log failures at WARNING or higher and expose integrity_updated in ExportResult and the JSON envelope. -
CR-A13-018 (code, Medium) — generate_stream has no cancellation: a timed-out or abandoned stream leaves the generate thread running, and the next turn runs a concurrent generate
severity Medium · reachabilitydefault· intentintended-but-harmful· confidence confirmed · independently confirmed · reproduced end to end · already tracked: KNOWN-OPEN CLI-08 — docs/roadmap/phase-16-trust-surface-hardening.md:475-476 (S6 open; unit id inferred)
Failure scenario. On CPU or MPS hardware the first long turn times out, and each later turn stacks another background generate. Output slows to a crawl or the process runs out of memory, and the timeout errors give no hint that earlier generations are still running.
Where.forgelm/inference.py:308-339,forgelm/chat.py:261-275(confirmed),forgelm/inference.py:291-296,forgelm/inference.py:310-339
Recommended fix. Pass a StoppingCriteria driven by a threading.Event that is set in a finally block around the consumer loop, then join the thread. Make the timeout configurable, or apply it only after the first token. -
CR-A13-019 (code, Medium) — Generate and generate_stream tokenize the rendered chat template with add_special_tokens=True, which duplicates BOS for BOS-adding tokenizers (Llama-3, Gemma, Mistral)
severity Medium · reachabilitydefault· intentcontract-violation· confidence confirmed · independently confirmed · reproduced end to end
Failure scenario.forgelm chaton a Gemma or Llama-3 fine-tune feeds ... on every turn. Responses degrade and the operator may conclude the fine-tune regressed.
Where.forgelm/inference.py:167-187,forgelm/inference.py:237-238,forgelm/inference.py:285-286,forgelm/chat.py:253-263
Recommended fix. Use apply_chat_template(..., tokenize=True, return_dict=True) on the template path, or call tokenizer(text, add_special_tokens=False) when a chat template was applied, and pass the attention_mask along. -
CR-A13-020 (code, Medium) — Chat history is trimmed by message count (50 pairs), not by token budget
severity Medium · reachabilitydefault· intentcontract-violation· confidence confirmed · independently confirmed · not reproduced (code trace)
Failure scenario. After several long pasted documents, every turn fails or the model sees a truncated context, with no history trimming applied.
Where.forgelm/chat.py:39-40(confirmed),forgelm/chat.py:242-249,docs/usermanuals/en/deployment/chat.md:128-130,forgelm/chat.py:40
Recommended fix. Trim by token count against tokenizer.model_max_length or the config's max_position_embeddings minus max_new_tokens. -
CR-A13-021 (code, Medium) — Native merge core (_linear_merge and the _ties_dare_merge per-key path) is never really executed by any test; a vacuous test passes on empty deltas
severity Medium · reachabilitydefault· intentintended-but-harmful· 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. A refactor that flips the sign of the applied delta, drops weight normalisation, or reports failures as success keeps the suite green, so a wrong merged model reaches users. The SLERP defect above is exactly this class and survived.
Where.tests/test_merging_algos.py:318-337,tests/test_merging_algos.py:280-292,forgelm/merging.py:115-158,forgelm/merging.py:338-357
Recommended fix. Add an end-to-end merge test on the tiny-qwen fixture with two real PEFT adapters, asserting numerics against an independent computation for linear, ties, dare (drop 0 and fixed seed) and slerp. Also assert on the failure path (success False plus error text). Delete or fix the vacuous test. -
CR-A13-022 (code, Medium) — trust_remote_code propagation is unpinned across inference, chat, merge, export and deploy: hard-coding True passes every test
severity Medium · reachabilitydefault· intentintended-but-harmful· confidence confirmed · independently confirmed · reproduced end to end
Failure scenario. A regression makes forgelm chat, forgelm export --adapter or forgelm --merge execute repository-bundled code for an untrusted Hub model (IN-07 is TC-U) by default. No test fails, and the stated guarantee loses its regression net in these entry points.
Where.forgelm/inference.py:87,forgelm/chat.py:367,forgelm/cli/_no_train_modes.py:146,forgelm/export.py:174
Recommended fix. Add a table test per entry point that asserts the exact trust_remote_code kwarg reaching AutoTokenizer/AutoModelForCausalLM.from_pretrained, for both the default (False) and an explicit True. For the export merge, assert False is always passed. -
CR-A13-023 (code, Medium) — Chat REPL writes untrusted model output to the terminal with control sequences intact (rich escape covers markup only; the plain fallback escapes nothing)
severity Medium · reachabilitydefault· intentintended-but-harmful· confidence confirmed · independently confirmed · reproduced with stubs
Failure scenario. A model fine-tuned on scraped data, or pulled from a third-party repo, emitsESC cor a DCS/OSC sequence in an answer. Inforgelm chatit resets or clears the operator's terminal to hide output, retitles it, or, piped through tee/less -R, writes the clipboard or plants a disguised hyperlink.
Where.forgelm/chat.py:77-90,forgelm/chat.py:98-111,forgelm/chat.py:263-272,forgelm/chat.py:290-296
Recommended fix. Strip C0/C1 control characters (keep \n and \t) from model text before printing, in both the rich and the plain paths. For example, filter with str.translate over the C0 range minus \n/\t, plus 0x7f-0x9f. -
CR-A13-025 (code, Medium) — No test runs a real PEFT SLERP merge, and the fake-PEFT SLERP tests stay green with the state_a aliasing bug (and with the base-restore line deleted)
severity Medium · reachabilitydefault· intentintended-but-harmful· confidence confirmed · independently confirmed · reproduced end to end
Failure scenario. M-A13-001 is present: SLERP returns the second adapter, and the suite still reports green. A future refactor of the restore-and-reload sequence (merging.py:196-206) would also pass unnoticed.
Where.tests/test_merging_algos.py:600-626,forgelm/merging.py:196-206,tests/test_phase7.py:62-75
Recommended fix. Add a tiny-model test: use tiny-qwen with two small LoRA adapters generated in the test, or the env/artifacts adapters. Compute the expected per-tensor SLERP from two independently loaded merge_and_unload() results. Assert that the output differs from both inputs and equals the expected value to within a tolerance. Also assert that state_a does not alias base_model's parameters. -
CR-A13-026 (code, Medium) — A merged model can never pass verify-integrity: no supported path writes model_integrity.json for it, yet the docs and SOP require one before deployment
severity Medium · reachabilitydocumented-opt-in· intentcontract-violation· confidence confirmed · independently confirmed · reproduced (plumbing only)
Failure scenario. An operator merges three approved specialist adapters with forgelm --merge (exit 0) and follows the SOP 4.4 checklist and the verify-integrity CI-gate advice. verify-integrity on the merged directory exits 1 every time. No forgelm command produces the manifest the docs say the merged variant needs, so the team either removes the gate or ships without the integrity record the SOP requires.
Where.forgelm/merging.py:98-100,forgelm/cli/_no_train_modes.py:128-177,forgelm/trainer.py:1504,forgelm/trainer.py:2157-2167
Recommended fix. Have --merge (and merge_peft_adapters, or the CLI wrapper) write model_integrity.json via generate_model_integrity into merge.output_dir, as the trainer does. Otherwise, expose a supported producer and fix verify-integrity.md:8/111/115 and SOP 4.4 to say how a merged or exported variant gets its manifest. Also correct 'the compliance export writes model_integrity.json': the trainer's finalisation… -
CR-A13-027 (code, Medium) — gguf-export.md documents a multi-quant provenance manifest (base_model, fine_tune_run_id, quants{}, exported_at) that nothing writes
severity Medium · reachabilitydefault· intentcontract-violation· confidence confirmed · independently confirmed · not reproduced (code trace)
Failure scenario. A deployer archives GGUF files expecting the documented manifest to link each quant to its fine-tune run id. No such file is written, so the provenance link from deployed GGUF to the audited training run exists only in the operator's memory.
Where.docs/usermanuals/en/deployment/gguf-export.md:22,docs/usermanuals/en/deployment/gguf-export.md:56-69(confirmed),forgelm/export.py:207-239(confirmed),forgelm/export.py:493-494
Recommended fix. Either write the documented manifest (with the run_id read from the model dir's compliance artefacts, plus a timestamp and source path) or replace the example with the real exported_artifacts shape. -
CR-A13-028 (code, Medium) — Deploy silently overwrites any file at the output path, including default-named operator files in the cwd and files inside the model directory, and writes through symlinks
severity Medium · reachabilitydefault· intentintended-but-harmful· confidence confirmed · independently confirmed · reproduced end to end · already tracked: KNOWN-OPEN CLI-14 — docs/roadmap/phase-16-trust-surface-hardening.md:626-627 (S11 open; unit id inferred from exit ordering)
Failure scenario. An operator runsforgelm deploy ./final_model --target tgifrom a repo root that already has a docker-compose.yaml for other services. The file is replaced without notice, exit 0.
Where.forgelm/deploy.py:309-311,forgelm/deploy.py:310,forgelm/deploy.py:285-286,forgelm/deploy.py:36-41
Recommended fix. Add an overwrite guard with an explicit --force flag (at minimum for the implicit default filename), reject output paths inside model_path, and log a warning when an existing file is replaced. -
CR-A13-030 (code, Medium) — --merge writes output_dir in place with no staging or cleanup: a failed merge leaves a directory that loads as a model, destroys or silently replaces a previous good merge, and the failure envelope n…
severity Medium · reachabilitydefault· intentintended-but-harmful· confidence confirmed · independently confirmed · reproduced with stubs
Failure scenario. The disk fills while a re-merge into ./merged_model is saving the tokenizer. The job exits 2, and the envelope does not say where it wrote. ./merged_model now holds complete weights from the failed run, so the previous good merge is gone, plus the old tokenizer. The next pipeline step (deploy/export/chat) loads it as a normal model. Variant: on a fresh directory the tokenizer is missing, load_model succeeds with a 1-token tokenizer, and every later generation dies with an opaque reshape error.
Where.forgelm/merging.py:110-112,forgelm/merging.py:28-36,forgelm/cli/_no_train_modes.py:152-177,forgelm/config.py:194
Recommended fix. Save into a sibling temporary directory (output_dir + '.tmp-'). Once both model and tokenizer are saved, swap it into place with os.replace, or rename the old directory aside first. Delete the staging directory on failure. Carry the configured output_dir and method on the failure MergeResult so the envelope names the target. Optionally write a model_integrity.json, or check for a tokenizer f… -
CR-A13-031 (code, Medium) — json-output.md 'canonical' reference drifts from emitted envelopes and has no guard
severity Medium · reachabilitydefault· intentcontract-violation· confidence confirmed · independently confirmed · reproduced end to end
Failure scenario. A consumer written to the common convention reads.erroron failure and gets null for every verify-* and gate failure, losing the failure reason in CI logs.
Where.docs/usermanuals/en/reference/json-output.md:16,docs/usermanuals/en/reference/json-output.md:176-186,docs/usermanuals/en/reference/json-output.md:863-890,forgelm/cli/subcommands/_export.py:33-35
Recommended fix. Either add 'error' to every success:false envelope or narrow json-output.md:16. Document the extra export keys. Add a guard test that runs each subcommand's JSON path and diffs keys against the page.
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