From 8a9c7828126ab23e6a019096739745a465c52c51 Mon Sep 17 00:00:00 2001
From: hiyouwhatsapp <89997383+hiyouwhatsapp@users.noreply.github.com>
Date: Sun, 18 Oct 2026 01:56:20 +0300
Subject: [PATCH] fix(cli): preserve root flags before subcommands

Suppress omitted subparser defaults so explicit safety flags and paths survive parsing. Cover both flag positions, omitted defaults and subcommand precedence.

Fixes #414

Resolves: CR-A05-001
---
 CHANGELOG.md               |  1 +
 forgelm/cli/_parser.py     | 15 +++++++----
 tests/test_cli.py          | 55 ++++++++++++++++++++++++++++++++++++++
 tools/check_module_size.py |  7 ++++-
 4 files changed, 72 insertions(+), 6 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 904500f6..976c967b 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -63,6 +63,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).
 - Fix the KTO bullet on the Alignment overview user-manual page (English and Turkish): it named the
   fields `(prompt, response, good?)`, but the KTO loader reads `prompt`, `completion` and a boolean
   `label` (#965).
diff --git a/forgelm/cli/_parser.py b/forgelm/cli/_parser.py
index 562c6343..be1c79ab 100644
--- a/forgelm/cli/_parser.py
+++ b/forgelm/cli/_parser.py
@@ -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(
@@ -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(
@@ -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) "
@@ -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).",
     )
@@ -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 > "
@@ -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.",
     )
@@ -1158,6 +1162,7 @@ def _add_purge_subcommand(subparsers) -> None:
     p.add_argument(
         "--dry-run",
         action="store_true",
+        default=argparse.SUPPRESS,
         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)
diff --git a/tests/test_cli.py b/tests/test_cli.py
index a9765895..311600e2 100644
--- a/tests/test_cli.py
+++ b/tests/test_cli.py
@@ -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
diff --git a/tools/check_module_size.py b/tools/check_module_size.py
index 35bb0b0d..6f63b52a 100644
--- a/tools/check_module_size.py
+++ b/tools/check_module_size.py
@@ -449,7 +449,7 @@ 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, "
@@ -457,6 +457,11 @@ class _DeferredSplit:
             "--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 "