fix(train): the three commits that missed the merge — including a failed run reporting success

src/praisonai-desktop/engine/test_portability.py

@@ -282,6 +282,9 @@ def setUp(self):
         os.environ["PATH"] = self.bin + os.pathsep + self.old_path
 
     def tearDown(self):
+        # setUp skips before these are set on Windows.
+        if getattr(self, "bin", None) is None:
+            return
         os.environ["PATH"] = self.old_path
         shutil.rmtree(self.bin, ignore_errors=True)
 

src/praisonai-train/praisonai_train/cli/commands/train.py

@@ -153,7 +153,7 @@ def _v(value):
         # the difference between a caught typo and a wasted run. Resolved and
         # printed WITHOUT loading the (heavy, optional) runner, so a preview
         # never depends on the training deps being installed.
-        _print_resolved_config(config, overrides)
+        _print_resolved_config(config, overrides, remote_overrides)
         return
 
     if not dataset and not config:
@@ -268,9 +268,14 @@ def _dispatch_remote(resolved, remote_overrides, config_path, dataset):
                           workdir=block["workdir"])
 
     shipped = _write_shipped_config(resolved)
+    # A dataset named only in --config still has to be copied. Fall back to the
+    # resolved dataset when the positional argument is absent, but only when it
+    # points at a local file -- a HuggingFace id or a path already on the remote
+    # host is not something to ship.
+    ship_dataset = dataset or _local_dataset_in(resolved)
     try:
         run = runner.start(config_path=shipped,
-                           dataset_path=Path(dataset) if dataset else None,
+                           dataset_path=Path(ship_dataset) if ship_dataset else None,
                            expect_gpus=block["gpus"])
     except RemoteError as exc:
         output.print_error(f"Could not start the run on {block['host']}",
@@ -290,11 +295,52 @@ def _dispatch_remote(resolved, remote_overrides, config_path, dataset):
     runner.tail(run, on_line=typer.echo)
     state = runner.status(run)
     typer.echo(f"status: {state}")
-    if state == "failed":
+    # status() returns "failed (exit N)", not a bare "failed", so an equality
+    # check would print the failure and then exit 0 -- reporting success for a
+    # run that did not complete.
+    if state.startswith("failed"):
         raise typer.Exit(1)
     return True
 
 
+def _local_dataset_in(resolved):
+    """The one local dataset file the resolved config names, or None.
+
+    The trainer accepts a dataset as a bare string or as the list-of-mappings
+    it normalises to -- `[{name: ...}]`, optionally with `data_files` -- and
+    loads any `name`/`data_files` that `os.path.exists` as a local file
+    (praisonai_train/train/llm/trainer.py:882). A remote run has to copy that
+    file, or the far side is handed a path that exists only on this machine.
+    Only the string form was covered before, so the canonical list form went
+    unshipped.
+
+    A HuggingFace id or a path already on the remote host is not a file here,
+    so it is left alone. Only the first local file is returned: the runner
+    ships a single positional dataset, which matches how a `--config` run is
+    launched.
+    """
+    entries = resolved.get("dataset")
+    if isinstance(entries, str):
+        entries = [entries]
+    elif not isinstance(entries, list):
+        return None
+
+    for entry in entries:
+        if isinstance(entry, str):
+            candidate = entry
+        elif isinstance(entry, dict):
+            # `data_files` is the explicit local file; `name` doubles as a path
+            # when it is one, exactly as the trainer treats it.
+            candidate = entry.get("data_files") or entry.get("name")
+            if isinstance(candidate, (list, tuple)):
+                candidate = candidate[0] if candidate else None
+        else:
+            continue
+        if isinstance(candidate, str) and Path(candidate).is_file():
+            return candidate
+    return None
+
+
 def _write_shipped_config(resolved):
     """The config to send, in a temp file. Returns its path.
 
@@ -370,11 +416,29 @@ def _resolve_config(config_path, overrides):
     return resolved
 
 
-def _print_resolved_config(config_path, overrides):
-    """Show the config the run would use: the file, then the flags on top."""
+def _print_resolved_config(config_path, overrides, remote_overrides=None):
+    """Show the config the run would use: the file, then the flags on top.
+
+    The remote block is resolved with the same precedence and validation as the
+    real dispatch, so the preview shows the host, interpreter, workdir and GPU
+    count the run would actually use -- and a bad remote setting is caught here
+    rather than after an hour of rented GPU.
+    """
     import yaml
 
+    from ..output.console import get_output_controller
+    from praisonai_train.remote import settings as remote_settings
+
     resolved = _resolve_config(config_path, overrides)
+
+    try:
+        block = remote_settings.resolve(resolved, remote_overrides or {})
+    except remote_settings.RemoteSettingsError as exc:
+        get_output_controller().print_error("Bad remote settings", remediation=str(exc))
+        raise typer.Exit(1) from exc
+    if block:
+        resolved["remote"] = remote_settings.redact(block)
+
     typer.echo(yaml.safe_dump(resolved, sort_keys=True, default_flow_style=False).rstrip())
     if overrides:
         typer.echo(f"\n# {len(overrides)} value(s) came from flags: "

src/praisonai-train/praisonai_train/remote/settings.py

@@ -55,11 +55,18 @@ def resolve(config: dict, overrides: dict) -> dict:
     wins. Returns {} when no host is settled, which is how "train locally" is
     expressed -- there is no separate mode switch to get out of step with it.
     """
-    block = config.get("remote") or {}
-    if not isinstance(block, dict):
+    # Only an omitted or null `remote` means "train locally". A falsey but
+    # present value -- [], "", false -- is a malformed block, not an absence,
+    # and `... or {}` would have quietly swallowed it into local training.
+    raw = config.get("remote")
+    if raw is None:
+        block = {}
+    elif not isinstance(raw, dict):
         raise RemoteSettingsError(
             "remote: must be a mapping of key: value, not "
-            f"{type(block).__name__}")
+            f"{type(raw).__name__}")
+    else:
+        block = raw
 
     merged = defaults()
     merged.update({k: v for k, v in block.items() if v is not None})

src/praisonai-train/tests/unit/test_remote_dispatch.py

@@ -28,7 +28,11 @@ def __init__(self, host, python, workdir):
         self.host, self.python, self.workdir = host, python, workdir
         self.started_with = None
         self.tailed = False
-        self.state = "done"
+        # The vocabulary the real runner speaks, not one invented here. It
+        # returned "done" before, and the dispatch compared against "failed"
+        # with ==, so a run that ended "failed (exit 1)" was reported as a
+        # success and the fake agreed with the bug.
+        self.state = "completed"
         FakeRunner.instances.append(self)
 
     def start(self, config_path=None, dataset_path=None, expect_gpus=1, **_):
@@ -105,6 +109,50 @@ def test_it_does_not_write_config_yaml_into_the_working_directory(
         assert not (tmp_path / "config.yaml").exists(), (
             "a remote run clobbered ./config.yaml")
 
+    def test_a_dataset_named_only_in_the_config_is_shipped(self, tmp_path):
+        # No positional dataset argument, but the resolved config names a local
+        # file. It must still be copied, or the remote process gets a path that
+        # exists only on this machine.
+        data = tmp_path / "d.json"
+        data.write_text("[]", encoding="utf-8")
+        _dispatch({"remote": {"host": "gpubox"}, "dataset": str(data)})
+        sent = FakeRunner.instances[0].started_with["dataset"]
+        assert sent is not None and pathlib.Path(sent) == data
+
+    def test_a_non_local_dataset_is_not_shipped(self, tmp_path):
+        # A HuggingFace id (or a path already on the far side) is not a file to
+        # copy.
+        _dispatch({"remote": {"host": "gpubox"}, "dataset": "org/dataset"})
+        assert FakeRunner.instances[0].started_with["dataset"] is None
+
+    def test_a_list_form_local_dataset_is_shipped(self, tmp_path):
+        # The trainer's canonical shape is a list of mappings, and a `name`
+        # that is a local file is loaded from disk. The string-only fallback
+        # missed it, so the file never reached the host and the run failed on a
+        # path that exists only here.
+        data = tmp_path / "d.json"
+        data.write_text("[]", encoding="utf-8")
+        _dispatch({"remote": {"host": "gpubox"},
+                   "dataset": [{"name": str(data)}]})
+        sent = FakeRunner.instances[0].started_with["dataset"]
+        assert sent is not None and pathlib.Path(sent) == data
+
+    def test_a_list_form_data_files_local_dataset_is_shipped(self, tmp_path):
+        # `data_files` names the local file explicitly, with `name` free to be
+        # a label. The trainer loads `data_files`, so it is what must ship.
+        data = tmp_path / "d.jsonl"
+        data.write_text("", encoding="utf-8")
+        _dispatch({"remote": {"host": "gpubox"},
+                   "dataset": [{"name": "my-set", "data_files": str(data)}]})
+        sent = FakeRunner.instances[0].started_with["dataset"]
+        assert sent is not None and pathlib.Path(sent) == data
+
+    def test_a_list_form_hub_dataset_is_not_shipped(self, tmp_path):
+        # A hub id in list form is not a local file; nothing to copy.
+        _dispatch({"remote": {"host": "gpubox"},
+                   "dataset": [{"name": "org/dataset"}]})
+        assert FakeRunner.instances[0].started_with["dataset"] is None
+
     def test_the_gpu_expectation_is_passed_through(self):
         _dispatch({"remote": {"host": "gpubox", "gpus": 4}})
         assert FakeRunner.instances[0].started_with["gpus"] == 4
@@ -117,13 +165,67 @@ def test_the_interpreter_and_workdir_are_passed_through(self):
         assert made.workdir == "~/runs"
 
 
+class TestTheStatusVocabulary:
+    """The fake must speak the language the real runner speaks."""
+
+    def test_the_fake_only_returns_states_the_runner_can_return(self):
+        # Read from disk, not through the module: the autouse fixture has
+        # already swapped RemoteRunner for the fake, so inspecting the
+        # attribute would have this test confirm the fake agrees with itself.
+        source = (pathlib.Path(__file__).resolve().parents[2]
+                  / "praisonai_train" / "remote" / "runner.py").read_text()
+        for word in ("completed", "running", "unknown"):
+            assert word in source, (
+                f"the runner no longer reports {word!r}; the fakes here are "
+                "built on that vocabulary")
+        assert "failed (exit" in source, (
+            "the runner no longer reports 'failed (exit N)' -- the dispatch "
+            "matches that shape by prefix")
+
+    @pytest.mark.parametrize("state", ["failed (exit 1)", "failed (exit 137)"])
+    def test_a_failed_run_is_a_failure_however_it_is_spelled(self, state, monkeypatch):
+        import typer
+
+        class Failing(FakeRunner):
+            def status(self, run):
+                return state
+
+        import praisonai_train.remote.runner as runner_mod
+        monkeypatch.setattr(runner_mod, "RemoteRunner", Failing)
+        with pytest.raises(typer.Exit):
+            _dispatch({"remote": {"host": "gpubox"}})
+
+    def test_a_completed_run_is_not_a_failure(self, monkeypatch):
+        class Completed(FakeRunner):
+            def status(self, run):
+                return "completed"
+
+        import praisonai_train.remote.runner as runner_mod
+        monkeypatch.setattr(runner_mod, "RemoteRunner", Completed)
+        assert _dispatch({"remote": {"host": "gpubox"}}) is True
+
+
 class TestFailure:
     def test_a_run_that_ends_failed_is_reported_as_failure(self, monkeypatch):
         import typer
 
         class Failing(FakeRunner):
             def status(self, run):
-                return "failed"
+                return "failed (exit 1)"
+
+        import praisonai_train.remote.runner as runner_mod
+        monkeypatch.setattr(runner_mod, "RemoteRunner", Failing)
+        with pytest.raises(typer.Exit):
+            _dispatch({"remote": {"host": "gpubox"}})
+
+    def test_a_failed_status_with_an_exit_code_is_reported_as_failure(self, monkeypatch):
+        # status() returns "failed (exit N)", not a bare "failed". An equality
+        # check matched neither and reported a failed run as success.
+        import typer
+
+        class Failing(FakeRunner):
+            def status(self, run):
+                return "failed (exit 1)"
 
         import praisonai_train.remote.runner as runner_mod
         monkeypatch.setattr(runner_mod, "RemoteRunner", Failing)
@@ -157,7 +259,8 @@ def test_llm_with_a_remote_config_never_reaches_the_local_trainer(
         def _boom(*_a, **_k):
             raise AssertionError("the local trainer was reached for a remote run")
 
-        monkeypatch.setattr(train_cmd, "import_code_module", _boom, raising=False)
+        import praisonai_train._code_bridge as bridge
+        monkeypatch.setattr(bridge, "import_code_module", _boom)
         train_cmd.train_llm(config=config)
 
         assert len(FakeRunner.instances) == 1, "the run was not sent anywhere"
@@ -174,13 +277,16 @@ def test_llm_without_remote_settings_does_not_dispatch(self, tmp_path, monkeypat
         monkeypatch.chdir(tmp_path)
         config = _config(tmp_path, {"model_name": "unsloth/tiny", "dataset": "d.json"})
         # Stop before the heavy import; the point is only that nothing was sent.
-        monkeypatch.setattr(train_cmd, "import_code_module",
-                            lambda *_a, **_k: (_ for _ in ()).throw(ImportError("no")),
-                            raising=False)
-        try:
+        import typer
+
+        import praisonai_train._code_bridge as bridge
+        monkeypatch.setattr(bridge, "import_code_module",
+                            lambda *_a, **_k: (_ for _ in ()).throw(ImportError("no")))
+        # The ImportError is surfaced as a typer.Exit; asserting it keeps this
+        # test honest -- a blind `except Exception` would pass even if the
+        # command failed for an unrelated reason before the local trainer.
+        with pytest.raises(typer.Exit):
             train_cmd.train_llm(config=config)
-        except Exception:
-            pass
         assert FakeRunner.instances == [], "a local run was sent to a remote host"
 
     def test_dry_run_shows_the_remote_block_and_sends_nothing(self, tmp_path, monkeypatch):

src/praisonai-train/tests/unit/test_remote_parity.py

@@ -116,3 +116,15 @@ def test_gpus_must_be_a_positive_integer(self):
     def test_a_non_mapping_remote_block_is_refused(self):
         with pytest.raises(settings.RemoteSettingsError):
             settings.resolve({"remote": "gpubox"}, {})
+
+    def test_a_falsey_non_mapping_remote_block_is_refused(self):
+        # [], "" and false are present but malformed. `remote: or {}` would
+        # have swallowed them into local training instead of flagging the
+        # mistake -- only an omitted or null value means "train here".
+        for bad in ([], "", False):
+            with pytest.raises(settings.RemoteSettingsError):
+                settings.resolve({"remote": bad}, {})
+
+    def test_an_omitted_or_null_remote_block_trains_here(self):
+        assert settings.resolve({}, {}) == {}
+        assert settings.resolve({"remote": None}, {}) == {}