Skip to content

[Wave 4] Low/info — Module sweep: CLI framework: parser, dispatch, exit-code contract, logging (part 1/3) #80

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-A05-026 (code, Low) — Standalone safety-eval writes no audit event on a completed run although --output-dir help promises an audit log
    severity Low · reachability default · intent intended-but-harmful · confidence likely · single reviewer · reproduced end to end
    Failure scenario. A regulated pipeline relies on the audit log for evidence of release-time safety checks; none exists for passing or failing runs (the evaluation_completed audit event exists only in the trainer path).
    Where. forgelm/cli/_parser.py:895-901, forgelm/cli/subcommands/_safety_eval.py:216-221 (confirmed), forgelm/cli/subcommands/_safety_eval.py:300-314 (confirmed), docs/reference/safety_eval_subcommand.md:75 (confirmed)
    Recommended fix. Emit safety.evaluation_completed (safety_audit_fields) from the subcommand, or correct the help text.

  • CR-A05-027 (code, Low) — --dry-run help and docstring promise a model/dataset access check the implementation never performs
    severity Low · reachability default · intent contract-violation · confidence likely · single reviewer · reproduced end to end
    Failure scenario. A CI pre-flight runs --dry-run on a config whose model and dataset paths do not exist. It gets exit 0 and success:true, and the real run fails later.
    Where. forgelm/cli/_parser.py:1350, forgelm/cli/_dry_run.py:53, forgelm/cli/_dry_run.py:72, forgelm/cli/_dry_run.py:40
    Recommended fix. Either implement cheap local checks (path exists for local model and dataset; a Hub check only when online and opted in) or reword the help and docstring to 'validate configuration only'.

  • CR-A05-028 (code, Low) — Deploy --port has no upper bound: --port 99999 writes an invalid compose port mapping and reports success
    severity Low · reachability default · intent intended-but-harmful · confidence likely · single reviewer · reproduced end to end · already tracked: KNOWN-OPEN CLI-16 — docs/roadmap/phase-16-trust-surface-hardening.md:628-630 (S11 open; unit id inferred)
    Failure scenario. The generated compose file fails only at docker compose up time.
    Where. forgelm/cli/_parser.py:130, forgelm/deploy.py:164
    Recommended fix. Add a port-range type (1-65535).

  • CR-A05-029 (code, Low) — Pipeline config hash comes from a second read of the YAML, not the bytes that were parsed
    severity Low · reachability default · intent intended-but-harmful · confidence likely · single reviewer · reproduced (plumbing only)
    Failure scenario. 'forgelm --config <(cat pipeline_v2.yaml) --resume-from dpo_stage' after a run started with '--config <(cat pipeline_v1.yaml)' passes the stale-state check because both hashes are sha256(b''). The manifest records a hash that matches no file.
    Where. forgelm/cli/_dispatch.py:199-210, forgelm/cli/_dispatch.py:242, forgelm/config.py:2245-2260, forgelm/cli/_pipeline.py:178-188
    Recommended fix. Read the bytes once, then parse the config from those same bytes (load_config_from_bytes) and hash that buffer. Refuse an empty buffer for a config that parsed to a pipeline.

  • CR-A05-030 (code, Low) — Ctrl-C exits 2 silently: no log line, no JSON envelope, and no terminal audit event or failure webhook for an interrupted run
    severity Low · reachability default · intent contract-violation · confidence confirmed · independently confirmed · reproduced with stubs
    Failure scenario. An operator presses Ctrl-C mid-training under 'forgelm --config x.yaml --output-format json | jq'. The process exits 2 with nothing on stdout and no explanatory stderr line. audit_log.jsonl ends at training.started with no terminal event, and the start webhook is never followed by a failure webhook.
    Where. forgelm/cli/_dispatch.py:120-121 (confirmed), forgelm/cli/_dispatch.py:168-169, forgelm/trainer.py:1693-1709 (except Exception: pipeline.failed + notify_failure, then raise), forgelm/cli/_training.py:173 (except Exception)
    Recommended fix. At the CLI seam, log 'Interrupted by user', emit {'success': false, 'error': 'interrupted'} in JSON mode, and have the training path record a terminal 'pipeline.failed'/'training.interrupted' audit event (cataloged) before re-raising or exiting.

  • CR-A05-031 (code, Low) — JSON mode and --quiet override an explicit --log-level, and basicConfig without force makes --log-level a no-op when handlers exist
    severity Low · reachability default · intent intended-but-harmful · confidence likely · independently confirmed · reproduced end to end
    Failure scenario. An operator debugging a CI job adds '--log-level DEBUG' to a JSON-mode invocation and gets no extra diagnostics. A notebook calling main() after logging.basicConfig() cannot change the verbosity.
    Where. forgelm/cli/_logging.py:21-33 (confirmed), forgelm/cli/_dispatch.py:134-136, forgelm/cli/_logging.py:25, forgelm/cli/_dispatch.py:239 (confirmed)
    Recommended fix. In JSON mode honour an explicit --log-level, since logs already go to stderr and stdout is unaffected, or at least document the override in cli.md and json-output.md. Pass force=True, or configure the forgelm logger rather than the root logger.

  • CR-A05-032 (code, Low) — torch/NumPy ABI preflight: stale numpy<2-marker narrative, a remediation (torch>=2.3) below the torch>=2.4 floor, and no preflight on the pipeline path
    severity Low · reachability default · intent contract-violation · confidence likely · independently confirmed · reproduced end to end
    Failure scenario. An operator with an out-of-band torch 2.2 follows the remediation, installs numpy<2, and still has a torch that transformers>=5.3 rejects. A pipeline-mode run on the same env skips the preflight entirely.
    Where. forgelm/cli/_abi_check.py:14-21 (confirmed), forgelm/cli/_abi_check.py:127-134 (confirmed), forgelm/cli/_training.py:39-56 (confirmed), pyproject.toml:46-52 (confirmed)
    Recommended fix. Update the docstrings. Change the remediation to 'upgrade torch to >=2.4 (ForgeLM's minimum)'. Call _preflight_numpy_torch_abi from the pipeline entry as well.

  • CR-A05-033 (code, Low) — UTF-8 console reconfigure is not a no-op on POSIX: it overrides PYTHONIOENCODING and turns surrogate-escaped bytes into '?'
    severity Low · reachability default · intent intended-but-harmful · confidence likely · independently confirmed · reproduced end to end
    Failure scenario. On Linux, a model directory containing a file named with byte 0xFF is listed as '?' in 'forgelm verify-integrity' text output, which hides which file changed.
    Where. forgelm/cli/_dispatch.py:125-153, forgelm/cli/_dispatch.py:165, forgelm/cli/subcommands/_verify_integrity.py:145-152
    Recommended fix. Limit the call to sys.platform == 'win32' (or to encodings that cannot encode the help text), keep surrogateescape on POSIX, and correct the docstring.

  • CR-A05-034 (code, Low) — Exit-code contract tests do not pin 3/4/5 membership in _PUBLIC_EXIT_CODES or the pipeline return pass-through (F-150)
    severity Low · reachability default · intent contract-violation · confidence confirmed · independently confirmed · reproduced end to end
    Failure scenario. A refactor of _PUBLIC_EXIT_CODES loses 4. A multi-stage pipeline paused for Art. 14 approval now exits 2 (training error) instead of 4, so CI approval routing breaks, and the suite is green.
    Where. forgelm/cli/_exit_codes.py:54-62, forgelm/cli/_dispatch.py:245-246, tests/test_cli.py:24-28, tests/test_cli_subcommands.py:237-267
    Recommended fix. Add a parametrised test over all of 0-6 asserting _clamp_exit_code is the identity. Add a test that patches run_pipeline_from_args to return 3, 4 and 5 and asserts main() exits with that exact code.

  • CR-A05-035 (code, Low) — Dispatch-table routing (approve/reject, purge, verify-gguf/integrity/annex-iv, ...) is not exercised through main(); swapped table entries survive the suite
    severity Low · reachability default · intent intended-but-harmful · confidence confirmed · independently confirmed · reproduced end to end
    Failure scenario. A copy-paste edit to the table silently routes forgelm reject to approve, or purge to reverse-pii, and CI is green.
    Where. forgelm/cli/_dispatch.py:76-96 (confirmed), forgelm/cli/_dispatch.py:84-96, forgelm/cli/subcommands/_verify_integrity.py:153-154, forgelm/cli/subcommands/_verify_integrity.py:144-152
    Recommended fix. Add a parametrised test that, for each of the 19 command names, patches the expected facade attribute and asserts it is the one invoked via main().

  • CR-A05-036 (code, Low) — --quiet / --log-level / JSON log suppression (F-152) have no behavioural test; the existing logging tests assert nothing
    severity Low · reachability default · intent intended-but-harmful · confidence likely · 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. -q stops silencing INFO output, or json mode starts leaking INFO lines onto the stream next to the JSON envelope, with no failing test.
    Where. tests/test_cli.py:39-47, forgelm/cli/_logging.py:21-33, forgelm/cli/_dispatch.py:172-198, forgelm/cli/_argparse_types.py:92-99
    Recommended fix. Test via subprocess or logging.basicConfig(force=True) and assert logging.getLogger().level for each flag combination in both flag positions.

  • CR-A05-037 (code, Low) — UTF-8 console reconfigure (F-203), the main() KeyboardInterrupt handler and the unknown-subcommand branch are untested
    severity Low · reachability default · intent intended-but-harmful · confidence likely · independently confirmed · reproduced end to end
    Failure scenario. Windows --help regresses to UnicodeEncodeError, or Ctrl-C exits 130 instead of 2, with no test failing.
    Where. forgelm/cli/_dispatch.py:97-100, forgelm/cli/_dispatch.py:125-153, forgelm/cli/_dispatch.py:165-169
    Recommended fix. Add unit tests with a fake stream exposing reconfigure, a SIGINT-raising dispatcher through main(), and the unknown-subcommand path.

  • CR-A05-038 (code, Low) — The --force-resume pipeline-only guard (F-026) is untested; the guard test loops over only three of the four flags
    severity Low · reachability default · intent intended-and-acceptable · confidence likely · independently confirmed · reproduced end to end
    Failure scenario. The flag is silently ignored on a single-stage run, which is the exact pre-fix behaviour the guard was added to remove.
    Where. forgelm/cli/_dispatch.py:253-274, tests/test_pipeline_cli.py:355-391
    Recommended fix. Add 'force-resume' to the parametrised flag set (no value argument).

  • CR-A05-039 (code, Low) — Parser contract mostly asserted only by import: required flags, mutually exclusive groups, defaults and numeric validators survive mutation
    severity Low · reachability default · intent intended-and-acceptable · confidence likely · single reviewer · reproduced end to end
    Failure scenario. approve without --output-dir, a verify-audit that ignores the HMAC env var name, or --temperature nan start passing parsing silently.
    Where. forgelm/cli/_parser.py:80, forgelm/cli/_parser.py:719, forgelm/cli/_parser.py:729, forgelm/cli/_parser.py:767
    Recommended fix. Add parametrised parser tests, one per required flag, mutex group and validator boundary (including nan/inf), and assert key defaults.

  • CR-A05-040 (code, Low) — test_offline_flag_sets_env is vacuous when HF_HUB_OFFLINE is preset and leaks os.environ on failure; the forgelm.cli module entry test cannot detect a missing sys.exit
    severity Low · reachability default · intent intended-but-harmful · confidence likely · independently confirmed · reproduced end to end
    Failure scenario. The --offline flag stops propagating to HF libraries while the only test stays green on any host where the variables are already set.
    Where. tests/test_cli.py:157-168, tests/test_main_module.py:211-221, forgelm/cli/__main__.py:20-22, forgelm/__main__.py:35-37
    Recommended fix. Use monkeypatch.delenv(..., raising=False) for the three variables before running main(), and in test_main_module assert exact exit codes for the module forms. Make forgelm/cli/main.py sys.exit(main()) to match forgelm/main.py.

  • CR-A05-041 (code, Low) — --strip-pattern ReDoS budget: guard works on POSIX main thread, but is applied per file with no circuit breaker, timeouts are not surfaced in exit code or summary, and the validator misses alternatio…
    severity Low · reachability documented-opt-in · intent intended-but-harmful · confidence likely (static only) · single reviewer · reproduced end to end
    Failure scenario. Operator ingests 2,000 scanned documents with a pattern like (\w+|\s)+: that the validator accepts. Each file whose text triggers backtracking burns 5 s: about 2.7 h of wall clock with exit 0, and some files are left unstripped without any machine-readable signal.
    Where. forgelm/cli/_parser.py:402-423, forgelm/_strip_pattern.py:240-290, forgelm/ingestion.py:565-571, forgelm/cli/subcommands/_ingest.py:123-125
    Recommended fix. Make the validator track every alternation branch. Add a per-run circuit breaker (after the first timeout for a pattern, skip it for the remaining files or abort with EXIT_CONFIG_ERROR). Record skipped_patterns / timeouts in the ingest result and notes_structured, and optionally exit non-zero. Consider the regex module's timeout argument or running the match in a killable worker so Windows is c…

  • CR-A05-042 (code, Low) — Subcommand dispatch seam is fail-open by construction (normal return => exit 0) and the 'clamp any other code to 2' promise holds only for two paths
    severity Low · reachability default · intent intended-but-harmful · confidence likely · single reviewer · not reproduced (code trace)
    Failure scenario. A refactor of a verify-* dispatcher replaces a failing sys.exit with return; the CLI reports exit 0 on a tampered artefact.
    Where. forgelm/cli/_dispatch.py:113-122, forgelm/cli/_exit_codes.py:55-62
    Recommended fix. Make every dispatcher return an int (like verify-audit) and have the seam do sys.exit(_clamp_exit_code(rc)) so a None return fails closed; or add a test asserting that each verify-* dispatcher's source cannot return normally after a failed verdict (NoReturn annotation plus mypy).

  • CR-A05-043 (code, Low) — Ingest --overlap is silently ignored (any value, including negative) under the paragraph strategy, contrary to the help
    severity Low · reachability default · intent contract-violation · confidence likely · single reviewer · reproduced end to end
    Failure scenario. Typo in a script is accepted without any signal.
    Where. forgelm/cli/_parser.py:213, forgelm/cli/_parser.py:225, forgelm/ingestion.py:1453 (refuted), forgelm/ingestion.py:2361
    Recommended fix. Use _non_negative_int for --overlap and a positive-int validator for --chunk-size.

  • CR-A05-044 (code, Low) — verify-annex-iv help and reference docs are out of step with the writer and pipeline behaviour (artefact name annex_iv_.json vs annex_iv_metadata.json)
    severity Low · reachability default · intent contract-violation · confidence likely · single reviewer · reproduced end to end
    Failure scenario. forgelm verify-annex-iv out/compliance/annex_iv_fg-abc.json gives exit 1 'not found'; the real file is out/compliance/annex_iv_metadata.json.
    Where. forgelm/cli/_parser.py:842, docs/reference/verify_annex_iv_subcommand.md:16 (confirmed), docs/reference/verify_annex_iv_subcommand-tr.md:16, forgelm/compliance.py:87
    Recommended fix. Change the help and both doc mirrors to compliance/annex_iv_metadata.json.

  • CR-A05-045 (code, Low) — --near-dup-threshold has no upper bound: >15 silently switches to O(n^2) brute force and >=64 marks every row pair a near-duplicate
    severity Low · reachability default · intent intended-but-harmful · confidence likely · independently confirmed · reproduced end to end
    Failure scenario. forgelm audit data/train.jsonl --near-dup-threshold 64 on 20 distinct rows -> pairs_per_split.train = 190 (= C(20,2)), success true, exit 0. On a 100K-row split the brute path builds about 5e9 pairs, which exhausts memory.
    Where. forgelm/cli/_parser.py:541-552, forgelm/data_audit/_orchestrator.py:295-296, forgelm/data_audit/_simhash.py:198-214, forgelm/data_audit/_simhash.py:290-292
    Recommended fix. Bound the flag (for example 0..15 so LSH stays exact, or < 64 with a warning above 15) and document the brute-force fallback.

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
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: cliCode area: clibacklog: 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