Skip to content

[Wave 4] Low/info — Module sweep: Post-training: inference, chat, GGUF export, deploy, merging (part 2/3) #706

Open

Description

@cemililik

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

Findings

  • CR-A13-056 (code, Low) — Deploy tests only check that output parses (yaml.safe_load / json.loads / substring); no upstream schema or parser validation, and one test pins the wrong escaping (G-A13-06, new)
    severity Low · reachability default · intent intended-but-harmful · confidence confirmed · independently confirmed · reproduced end to end
    Failure scenario. A change to emit an invalid vLLM key, a compose file TGI rejects, or an HF-Endpoints body the API rejects keeps every test green because the tests only reparse what the generator wrote; fixing the Ollama SYSTEM escaping would turn test_deploy.py:60 red.
    Where. tests/test_deploy.py:93-99, tests/test_deploy.py:73-77, tests/test_deploy.py:94, tests/test_deploy.py:123-145
    Recommended fix. Add schema/parser-backed golden tests: a minimal Modelfile grammar check (or fixture from upstream docs), a JSON Schema for the HF create-endpoint body, key-set checks against vLLM's serve config and TGI launcher flags, and flip test_system_prompt_with_quotes_escaped to the correct Ollama quoting.

  • CR-A13-057 (code, Low) — Adapter-only final_model loads through transformers' automatic PEFT path with the base taken from adapter_config.json unpinned, and the trainer never records the base revision in adapter_config.json
    severity Low · reachability default · intent intended-but-harmful · confidence likely · independently confirmed · reproduced end to end
    Where. forgelm/inference.py:80-105, forgelm/model.py:567-572, forgelm/model.py:609, forgelm/trainer.py:1711-1730
    Recommended fix. Pass revision= to get_peft_model so that adapter_config.json records the base revision. In load_model, detect adapter_config.json and load the base explicitly with that revision (as _resolve_benchmark_load_target already does for the path), and log the resolved base and revision.

  • CR-A13-058 (code, Low) — Forgelm export hands an unvetted model dir to llama.cpp's converter, which (per upstream, unverified offline) loads tokenizer code with trust_remote_code=True for some architectures; ForgeLM does not…
    severity Low · reachability default · intent intended-but-harmful · confidence likely (static only) · independently confirmed · reproduced with stubs
    Failure scenario. An operator exports a third-party or colleague-supplied model directory, or a fine-tune of a Qwen-1 / ChatGLM base, with FORGELM_GGUF_CONVERTER pointing at a llama.cpp checkout. That is the only working converter route, because default discovery inside llama-cpp-python fails (already reported). The upstream converter imports tokenization_*.py from the directory, so attacker code runs with the operator's privileges and environment, and ForgeLM reports a successful export.
    Where. forgelm/export.py:295-310 (_run_converter subprocess.run, no env scrubbing), forgelm/export.py:281-292 (_build_converter_command), forgelm/export.py:373-483 (export_model; converter lookup 415, optional merge 427-433, command build 443, run 446), forgelm/export.py:113-116 (converter-path warning only)
    Recommended fix. Short term: document in gguf-export.md, cli.md and the enterprise checklist that the llama.cpp converter can execute repository code for some architectures, so trust_remote_code=false does not cover forgelm export. Before spawning the converter, refuse or warn loudly (with an explicit opt-in flag) when the source dir contains auto_map in config.json or tokenizer_config.json, or any *.py file. O…

  • CR-A13-059 (code, Low) — _run_converter passes the full parent environment (HF_TOKEN, FORGELM_AUDIT_SECRET, webhook/API keys) to the converter subprocess, which needs none of it
    severity Low · reachability default · intent intended-but-harmful · confidence likely · independently confirmed · reproduced end to end
    Failure scenario. Repository code run by the converter reads FORGELM_AUDIT_SECRET from its environment. With the key, it can later rewrite the audit log and recompute a valid HMAC chain, and verify-audit then returns exit 0 on a tampered log.
    Where. forgelm/export.py:322-328, forgelm/export.py:346-351
    Recommended fix. Pass an allow-listed env to the converter (PATH, HOME, PYTHONPATH/virtualenv variables, locale, HF_HUB_OFFLINE/TRANSFORMERS_OFFLINE, HF_HOME/HF_HUB_CACHE). Drop FORGELM_AUDIT_SECRET, HF_TOKEN/HUGGINGFACE_TOKEN, *_API_KEY and webhook variables. Set HF_HUB_OFFLINE=1 and TRANSFORMERS_OFFLINE=1 for a local-dir conversion.

  • CR-A13-060 (code, Low) — gguf-export.md tells users to run llama.cpp's ./main, a binary renamed to llama-cli in June 2024
    severity Low · reachability default · intent contract-violation · confidence likely · single reviewer · not reproduced (code trace)
    Failure scenario. A user with a current llama.cpp build runs ./main and gets 'not found' or a deprecation stub.
    Where. docs/usermanuals/en/deployment/gguf-export.md:104, docs/usermanuals/tr/deployment/gguf-export.md:105, forgelm/export.py:470
    Recommended fix. Use llama-cli -m model.q4_k_m.gguf -p ... -n 256 in EN and TR.

  • CR-A13-061 (code, Low) — chat.md minor drift: 'set seed: in YAML', fictional REPL transcript, and 'same Llama Guard model used here'
    severity Low · reachability default · intent contract-violation · confidence likely · single reviewer · reproduced (plumbing only)
    Failure scenario. A user seeking reproducible chat output adds seed: to a YAML. The config is rejected and chat behaviour does not change.
    Where. docs/usermanuals/en/deployment/chat.md:125, docs/usermanuals/en/deployment/chat.md:14-24, docs/usermanuals/en/deployment/chat.md:97, docs/usermanuals/en/deployment/chat.md:139
    Recommended fix. Drop the seed advice, or say chat has no seed control. Regenerate the transcript from the real REPL. Reword the see-also line.

  • CR-A13-062 (code, Low) — Forgelm deploy truncates an existing config before writing, so a failed run leaves a zero-byte or partial file in place of the previous valid one
    severity Low · reachability default · intent intended-but-harmful · confidence confirmed · independently confirmed · reproduced end to end · already tracked: KNOWN-OPEN CLI-19 — docs/roadmap/phase-16-trust-surface-hardening.md:474-475 (S6 open; unit id inferred)
    Failure scenario. An operator regenerates a working Modelfile with a new system prompt. The run fails: here on encoding, and equally on a full disk. The previously working Modelfile is now empty, and the operator gets no message about it.
    Where. forgelm/deploy.py:309-311, forgelm/deploy.py:316-330
    Recommended fix. Write to a sibling temporary file in the same directory, fsync it, then os.replace it onto output_path, the same pattern used for the other atomic writers. Remove the temporary file on failure.

  • CR-A13-063 (code, Low) — CLAUDE.md 'no env-var sniffing for behaviour (only for secrets)' is contradicted by several documented and undocumented env toggles
    severity Low · reachability default · intent contract-violation · confidence likely · independently confirmed · reproduced end to end
    Failure scenario. A reviewer or agent trusts the rulebook and assumes a run is fully determined by its YAML. Yet the same YAML gives different provenance (no hf_revision under HF_HUB_OFFLINE=1), a different converter, or an anonymous-operator audit trail depending on ambient env. Also, FORGELM_AUDIT_TEST_WORKER_RAISES= left in an environment makes a production audit raise RuntimeError for that split.
    Where. CLAUDE.md:114, AGENTS.md:114, forgelm/export.py:100, forgelm/compliance.py:219
    Recommended fix. Restate the principle as 'secrets, identity and standard HF/XDG env vars, enumerated in the env-var reference', keep that list complete (add FORGELM_ALLOW_AUDIT_REROOT), and move the test-only FORGELM_AUDIT_TEST_WORKER_RAISES hook out of the production path, for example with a test-installed initializer.

  • CR-A13-064 (code, Low) — Deploy --target ollama rejects a .gguf file and Ollama registry names, and blames HF Hub IDs
    severity Low · reachability default · intent intended-but-harmful · confidence confirmed · single reviewer · reproduced end to end
    Failure scenario. forgelm deploy ./model.gguf --target ollama -> exit 1 with "'./model.gguf' is not a directory ... HF Hub IDs are not supported".
    Where. forgelm/deploy.py:270-283
    Recommended fix. For ollama, accept an existing *.gguf file as well as a directory, and word the error by the actual cause.

  • CR-A13-065 (code, Low) — No test pins the stopping ids (or any generate kwarg) of generate/generate_stream
    severity Low · reachability default · intent intended-and-acceptable · confidence likely · independently confirmed · reproduced end to end
    Failure scenario. Any change to the stopping ids, whether the current override, dropping eos entirely or a nonsense id, ships green.
    Where. tests/test_inference.py:575-608, forgelm/inference.py:240-246, forgelm/inference.py:297-304, tests/test_inference.py:598
    Recommended fix. Add a tiny-model test that loads a fixture with a multi-id generation_config and asserts the effective EosTokenCriteria ids, or that a forced end-of-turn stops generation, for generate and generate_stream. Make the safety path assert the same ids, so that cross-surface parity is pinned.

  • CR-A13-066 (code, Low) — No automated test exercises the train -> chat handoff on a real adapter-only final_model directory
    severity Low · reachability default · intent intended-and-acceptable · confidence likely · independently confirmed · reproduced end to end
    Where. forgelm/inference.py:99-105, forgelm/inference.py:109, forgelm/cli/subcommands/_quickstart.py:170, forgelm/trainer.py:1711-1730
    Recommended fix. Add an offline test that builds a tiny LoRA adapter directory (the tiny-qwen fixture style, no network) and asserts load_model(adapter_dir) with no adapter argument returns a model whose logits differ from the base; or make load_model detect adapter_config.json explicitly and route through PeftModel.from_pretrained so the behaviour is ForgeLM's own and documented.

  • CR-A13-067 (code, Low) — Adapter-only default: ForgeLM's own documented serving routes merge the adapter into a half-precision or requantised base, so on a bf16 or 4-bit base the served weights differ from those the gate sco…
    severity Low · reachability default · intent intended-but-harmful · confidence likely · independently confirmed · reproduced end to end
    Failure scenario. A run with a bf16 checkpoint, or the default QLoRA on CUDA, passes the safety gate on the unmerged in-memory model. The operator follows the documented routes (forgelm chat against the adapter, or forgelm export) and serves merged weights in bf16, fp16 or requantised NF4. On the probe set those weights produce different text from what the gate scored, and nothing in the safety results or the audit chain says what precision or form was certified.
    Where. forgelm/inference.py:109-115, forgelm/inference.py:109-114, forgelm/export.py:172-177, forgelm/model.py:440-450
    Recommended fix. Record the evaluated form in the gate evidence: dtype, quantisation config, and merged vs unmerged. Make chat/inference reuse the run's bnb settings. Document that a merged or exported model in reduced precision is a different model and has to be re-gated, as the merging guide already says for --merge. Optionally, offer to gate the merged form when merged serving is the intended route.

  • CR-A13-068 (code, Low) — Chat REPL with --system on a chat template that rejects the system role fails on every turn, with no fallback and no ForgeLM hint (and a misleading error on Mistral-v0.1-style templates)
    severity Low · reachability default · intent intended-but-harmful · confidence likely · independently confirmed · reproduced end to end
    Failure scenario. An operator fine-tunes a Gemma-2 base and runs forgelm chat ./final_model --system "You are a support agent". Every message prints '[Generation error: System role not supported]', and the REPL keeps prompting. The operator cannot get a single response and, because of the next finding, cannot fix it without restarting the session.
    Where. forgelm/chat.py:242-250, forgelm/chat.py:245, forgelm/inference.py:177-178, forgelm/inference.py:178
    Recommended fix. Catch jinja2 TemplateError in _build_prompt or in ChatSession. On a system-role rejection, either fold the system text into the first user message (with a one-time warning) or print an actionable hint: the template rejects the system role; use /system off or restart without --system. Add a caveat to chat.md (EN and TR).

  • CR-A13-069 (code, Low) — The REPL cannot clear a system prompt: /system with no argument only displays it, '/system none' sets the literal text 'none', and /reset keeps it
    severity Low · reachability default · intent intended-but-harmful · confidence confirmed · single reviewer · reproduced end to end
    Failure scenario. The operator starts forgelm chat model --system X on a Gemma-family model. Every turn fails, and they try /system, /reset and /system none. Nothing removes the system message, so the session cannot recover.
    Where. forgelm/chat.py:212, forgelm/chat.py:213, forgelm/chat.py:217, forgelm/chat.py:171
    Recommended fix. Support clearing, for example '/system --clear' or '/system off', and document it in /help and in chat.md (EN and TR). Consider printing the active system prompt in the generation-error hint.

  • CR-A13-070 (code, Low) — load_model with device_map='auto' plus an adapter fails with an opaque peft error once accelerate offloads any layer to disk (CPU offload and device splits merge correctly)
    severity Low · reachability documented-opt-in · intent intended-but-harmful · confidence likely (static only) · independently confirmed · reproduced end to end
    Failure scenario. An operator runs forgelm chat --adapter or --benchmark-only on a base too large for GPU plus CPU memory. accelerate places layers on disk, and loading the adapter crashes with KeyError 'base_model.model.model.model.embed_tokens'. Nothing tells the operator that disk offload is the cause.
    Where. forgelm/inference.py:48, forgelm/inference.py:88-89, forgelm/inference.py:113, forgelm/chat.py:361
    Recommended fix. After from_pretrained, check whether 'disk' appears in model.hf_device_map when an adapter is given. If it does, fail with an actionable error that suggests more memory or a pre-merged model, or keep the unmerged PeftModel. Document the limitation.

  • CR-A13-071 (code, Low) — Sibling of M-A13-023: --merge writes the base tokenizer and base config, so a ForgeLM-trained adapter's pad_token and pad_token_id are dropped
    severity Low · reachability default · intent intended-but-harmful · confidence likely · independently confirmed · reproduced end to end
    Failure scenario. The base model has no pad token. An operator trains with ForgeLM, so the adapter directory carries pad=eos, then runs --merge. The merged model directory has no pad_token in the tokenizer and pad_token_id null in config.json. Non-ForgeLM consumers that batch-pad, and a GGUF export made from it, get no pad id. A third-party adapter's chat template is dropped the same way.
    Where. forgelm/merging.py:74, forgelm/merging.py:100, forgelm/cli/_no_train_modes.py:137, forgelm/model.py:398-402
    Recommended fix. Fix this together with M-A13-023 in one helper. Pick the tokenizer from the first adapter directory that has a tokenizer_config.json, using _pick_tokenizer_source, and warn or fail if the adapters' tokenizers disagree. Sync pad_token_id into the merged config as model.py:591-596 does.

  • CR-A13-072 (code, Low) — Pipeline guide lists model.chat_template as an inherited model field, but ModelConfig forbids it
    severity Low · reachability default · intent contract-violation · confidence likely · single reviewer · reproduced end to end
    Failure scenario. An operator adds model.chat_template to a pipeline config as the guide implies, and config validation fails.
    Where. docs/guides/pipeline.md:105, docs/guides/pipeline-tr.md:106
    Recommended fix. Remove chat_template from the guide row in both mirrors, or add the field if it is planned.

  • CR-A13-073 (code, Low) — Model-supplied chat templates render in-process with no time or memory bound. The Jinja sandbox caps range() but nothing else, and ForgeLM adds no bound, size check or note
    severity Low · reachability default · intent intended-but-harmful · confidence likely · independently confirmed · reproduced end to end
    Failure scenario. An operator runs forgelm chat <hub-id> or uses a Hub teacher or guard whose tokenizer_config/chat_template.jinja contains {% for i in range(100000) %}{% for j in range(100000) %}... or {{ 'A' * 10**10 }}. The first render hangs the REPL, the synthetic-data run or the safety gate, or exhausts host memory or swap. No exit code or verdict is forged: the gate never completes, so it never reports a pass.
    Where. forgelm/inference.py:177-178, forgelm/inference.py:238-239, forgelm/inference.py:295, forgelm/synthetic.py:263-264
    Recommended fix. Treat this as an accepted upstream property and document it. In the security checklist, state that trust_remote_code=false does not stop repo-supplied chat templates from running as sandboxed Jinja, with no CPU or memory limits. Cheap hardening: compare the rendered prompt's length or token count with tokenizer.model_max_length (or a configured cap) before tokenizing and generating, and fail with…

  • CR-A13-074 (code, Low) — User manuals and the post-training notebook teach direct imports from modules the library-API contract classifies Internal ('reach-in is unsupported')
    severity Low · reachability default · intent contract-violation · confidence confirmed · independently confirmed · reproduced end to end
    Failure scenario. An integrator builds a nightly merge pipeline from model-merging.md. A later minor release changes merge_peft_adapters, which is Internal and has no deprecation cadence, and the pipeline breaks with no DeprecationWarning. Separately, a failed merge returns success=False, the copied snippet never checks it, and the pipeline goes on to evaluate or ship a stale output_dir.
    Where. docs/design/library_api.md:120-124, docs/reference/library_api_reference.md:27-29, docs/usermanuals/en/deployment/model-merging.md:128-145, docs/usermanuals/tr/deployment/model-merging.md:128-145
    Recommended fix. Decide the tier: either promote merge_peft_adapters, MergeResult, load_model, generate, generate_stream and estimate_vram to Experimental (re-export plus reference-doc rows), or replace the snippets with CLI invocations. Add if not result.success: raise ... to the merging snippet. Extend check_library_api_doc.py to flag reach-ins in manuals and notebooks.

  • CR-A13-075 (code, Low) — Export rewrites model_integrity.json, a dumps_strict artefact, with plain json.dump outside the guard's scope, and round-trips bare NaN/Infinity
    severity Low · reachability default · intent intended-but-harmful · confidence likely · independently confirmed · reproduced end to end · already tracked: CLAIMED-FIXED-STILL-PRESENT TRUST-15 — docs/roadmap/phase-16-trust-surface-hardening.md:9-11,339-343 (S2 done); CHANGELOG.md:91-101
    Where. forgelm/export.py:229, forgelm/trainer.py:2165-2166, tools/check_strict_json_writers.py:42-54
    Recommended fix. Write through dumps_strict (and tmp+os.replace), and add forgelm/export.py to _WRITER_MODULES so both writers of the artefact follow the same contract.

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.

Activity

  1. added
    bugSomething isn't working
    backlog: knownAlready tracked in docs/roadmap
    securitySecurity-relevant
    severity: lowMinor defect, inaccuracy or rough edge
    source: review-2026-10Found by the October 2026 code and documentation review
    wave: 4Roadmap wave 4: low / info / watch, when touching the module
    on Oct 9, 2026
  2. cemililik commented on Oct 9, 2026

    @cemililik
    CollaboratorAuthor

    Progress on development — verified 2026-10-09 at 08f2426.

    Fixed (ticked in the list above):

    • CR-A13-075 — export writes model_integrity.json through dumps_strict, serialised before the truncating open (08f2426)

    Everything else in this issue is unchanged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    area: post-trainingCode area: post-trainingbacklog: knownAlready tracked in docs/roadmapbugSomething isn't workingsecuritySecurity-relevantseverity: lowMinor defect, inaccuracy or rough edgesource: review-2026-10Found by the October 2026 code and documentation reviewwave: 4Roadmap wave 4: low / info / watch, when touching the module

    Type

    No type

    Projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions