Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ All notable changes to ForgeLM are documented here.

### Fixed

- Preserve top-level `--dry-run`, `--config`, `--offline` and `--output` values when a subcommand omits the same flag (CR-A05-001, #414).
- Stop describing the human-approval gate as enforcing segregation of duties (English and Turkish): it records the
trainer and approver identities and does not compare them, and both are self-asserted. The ISO 27001 and SOC 2
mappings, the Statement of Applicability, the change-management SOP, the deployer guide and the user manual now
Expand Down
15 changes: 10 additions & 5 deletions forgelm/cli/_parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,9 @@ def _add_deploy_subcommand(subparsers) -> None:
choices=["ollama", "vllm", "tgi", "hf-endpoints"],
help="Target serving runtime.",
)
p.add_argument("--output", type=str, default=None, metavar="FILE", help="Output file path (default: auto).")
p.add_argument(
"--output", type=str, default=argparse.SUPPRESS, metavar="FILE", help="Output file path (default: auto)."
)
p.add_argument("--system", type=str, default=None, metavar="PROMPT", help="System prompt (Ollama only).")
p.add_argument("--max-length", type=_positive_int, default=4096, help="Context window length (default: 4096).")
p.add_argument(
Expand Down Expand Up @@ -174,13 +176,14 @@ def _add_quickstart_subcommand(subparsers) -> None:
p.add_argument(
"--output",
type=str,
default=None,
default=argparse.SUPPRESS,
metavar="FILE",
help="Where to write the generated YAML (default: ./configs/<template>-<timestamp>.yaml).",
)
p.add_argument(
"--dry-run",
action="store_true",
default=argparse.SUPPRESS,
help="Generate the config and print next steps; do not invoke training.",
)
p.add_argument(
Expand Down Expand Up @@ -484,6 +487,7 @@ def _add_doctor_subcommand(subparsers) -> None:
p.add_argument(
"--offline",
action="store_true",
default=argparse.SUPPRESS,
help=(
"Skip the HuggingFace Hub network probe. Inspects the local HF cache "
"(precedence: HF_HUB_CACHE > HF_HOME/hub > ~/.cache/huggingface/hub) "
Expand Down Expand Up @@ -1007,7 +1011,7 @@ def _add_cache_models_subcommand(subparsers) -> None:
p.add_argument(
"--output",
type=str,
default=None,
default=argparse.SUPPRESS,
metavar="DIR",
help="Cache directory override (default: HF_HUB_CACHE > HF_HOME/hub > ~/.cache/huggingface/hub).",
)
Expand Down Expand Up @@ -1048,7 +1052,7 @@ def _add_cache_tasks_subcommand(subparsers) -> None:
p.add_argument(
"--output",
type=str,
default=None,
default=argparse.SUPPRESS,
metavar="DIR",
help=(
"Cache directory override (default: HF_DATASETS_CACHE > HF_HOME/datasets > "
Expand Down Expand Up @@ -1144,7 +1148,7 @@ def _add_purge_subcommand(subparsers) -> None:
p.add_argument(
"--config",
type=str,
default=None,
default=argparse.SUPPRESS,
metavar="PATH",
help="YAML config (optional in row/run mode; required for --check-policy). Detects the webhook block + retention horizons.",
)
Expand All @@ -1158,6 +1162,7 @@ def _add_purge_subcommand(subparsers) -> None:
p.add_argument(
"--dry-run",
action="store_true",
default=argparse.SUPPRESS,
Comment thread
cemililik marked this conversation as resolved.
help="Show what would be deleted; do not modify disk. Audit events still emit (with dry_run=True) so the chain reflects intent.",
)
_add_common_subparser_flags(p, include_output_format=True)
Expand Down
55 changes: 55 additions & 0 deletions tests/test_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -281,6 +281,61 @@ def test_single_mode_flag_still_accepted(self, tmp_path, minimal_config):
assert exc_info.value.code == EXIT_SUCCESS


class TestSharedSubcommandFlags:
@pytest.mark.parametrize(
"command,extra,flag,value,expected",
[
("purge", [], "--dry-run", [], True),
("purge", [], "--config", ["policy.yaml"], "policy.yaml"),
("quickstart", [], "--dry-run", [], True),
("doctor", [], "--offline", [], True),
("deploy", ["model", "--target", "ollama"], "--output", ["serving"], "serving"),
("quickstart", [], "--output", ["config.yaml"], "config.yaml"),
("cache-models", ["--model", "org/model"], "--output", ["cache"], "cache"),
("cache-tasks", ["--tasks", "hellaswag"], "--output", ["cache"], "cache"),
],
)
@pytest.mark.parametrize("before", [True, False], ids=["before-subcommand", "after-subcommand"])
def test_explicit_flag_is_preserved(self, command, extra, flag, value, expected, before):
from forgelm.cli._parser import parse_args

option = [flag, *value]
args = [*option, command, *extra] if before else [command, *extra, *option]
with patch("sys.argv", ["forgelm", *args]):
result = parse_args()

assert getattr(result, flag.removeprefix("--").replace("-", "_")) == expected

@pytest.mark.parametrize(
"args,attribute,expected",
[
(["purge"], "dry_run", False),
(["purge"], "config", None),
(["quickstart"], "dry_run", False),
(["doctor"], "offline", False),
(["deploy", "model", "--target", "ollama"], "output", None),
(["quickstart"], "output", None),
(["cache-models", "--model", "org/model"], "output", None),
(["cache-tasks", "--tasks", "hellaswag"], "output", None),
],
)
def test_omitted_flags_keep_their_defaults(self, args, attribute, expected):
from forgelm.cli._parser import parse_args

with patch("sys.argv", ["forgelm", *args]):
result = parse_args()

assert getattr(result, attribute) == expected

def test_subcommand_value_overrides_root_value(self):
from forgelm.cli._parser import parse_args

with patch("sys.argv", ["forgelm", "--output", "root.yaml", "quickstart", "--output", "local.yaml"]):
result = parse_args()

assert result.output == "local.yaml"


class TestParserPublicApi:
def test_parse_args_has_namespace_return_annotation(self):
"""Finding 8: ``parse_args`` is the one function in _parser imported and
Expand Down
7 changes: 6 additions & 1 deletion tools/check_module_size.py
Original file line number Diff line number Diff line change
Expand Up @@ -449,14 +449,19 @@ class _DeferredSplit:
),
),
"forgelm/cli/_parser.py": _DeferredSplit(
budget=1372,
budget=1375,
deferred_at_loc=1320,
reason=(
"Argparse wiring for the full CLI surface. Split candidates: _train, "
"_inspect, _data, _run. Low behavioural risk but every subcommand's "
"--help text is pinned by check_cli_help_consistency.py."
),
budget_history=(
"2026-10-18 (#414, 1372 -> 1375, +3): Preserve explicit root flags when "
"a subcommand omits them. The three boolean flags need SUPPRESS defaults "
"and deploy's longer output default wraps under ruff. Keep these defaults "
"beside their existing argument declarations; the deferred CLI split is "
"a separate concern.",
"2026-07-20: 1320 -> 1332 (+12) for `safety-eval "
"--max-safety-regression` plus the import of its default constant. The "
"safety gate this flag drives was already live and already reachable at "
Expand Down
Loading