diff --git a/.github/workflows/test-unit.yml b/.github/workflows/test-unit.yml index fc2713228..dec94a85c 100644 --- a/.github/workflows/test-unit.yml +++ b/.github/workflows/test-unit.yml @@ -41,6 +41,6 @@ jobs: python -m pip install --upgrade pip python -m pip install . - - name: Run unit tests to chech whether invalid meta is getting indexed or not + - name: Run unit tests (meta validation, cache operations, typo detection) run: | python -m unittest discover -s tests -p "test_*.py" -v diff --git a/README.md b/README.md index 449c972bb..59dc98f3c 100644 --- a/README.md +++ b/README.md @@ -18,6 +18,14 @@ On February 9, 2025, MLCFlow released its first stable version, 1.0.0. ### Key Features Building upon the core idea of CMind—wrapping native scripts with Python wrappers and YAML metadata—MLCFlow focuses exclusively on key automation components: **Scripts**, along with its complementary modules: **Cache**, **Docker**, and **Experiments**. This targeted design simplifies both implementation and interface, enabling a more user-friendly experience. +- **Typo detection** — Mistyped actions or targets produce a "Did you mean …?" hint before the error message, so mistakes are quick to correct: + ``` + $ mlc rune script + Did you mean 'run'? + mlc: error: argument command: invalid choice: 'rune' … + ``` + See [docs/typo_detection.md](docs/typo_detection.md) for details. + --- ### Status diff --git a/docs/typo_detection.md b/docs/typo_detection.md new file mode 100644 index 000000000..080e882ca --- /dev/null +++ b/docs/typo_detection.md @@ -0,0 +1,125 @@ +# Typo Detection ("Did you mean…?") + +MLCFlow detects mistyped commands and targets and suggests the closest valid +alternative before showing the standard error. + +## How it works + +When you type an invalid action or target, the CLI now prints a suggestion +immediately after the usage line: + +``` +$ mlc rune script + +usage: mlc [-h] {run,pull,test,add,...} ... + +Did you mean 'run'? + +mlc: error: argument command: invalid choice: 'rune' (choose from 'run', 'pull', ...) +``` + +``` +$ mlc run scrip + +usage: mlc run [-h] {repo,repos,script,cache} ... + +Did you mean 'script'? + +mlc: error: argument target: invalid choice: 'scrip' (choose from 'repo', 'repos', 'script', 'cache') +``` + +When several alternatives are similarly close: + +``` +$ mlc find rep + +Did you mean one of: 'repo', 'repos'? +``` + +If the input has no close match (e.g. a completely unrelated word), no hint is +shown — only the standard argparse error. + +## Typos detected + +Suggestions are shown for **both** levels of the command syntax: + +| Typo location | Example input | Suggestion shown | +|---|---|---| +| Action | `mlc rune script` | `Did you mean 'run'?` | +| Action | `mlc fnd script` | `Did you mean 'find'?` | +| Action | `mlc serach cache` | `Did you mean 'search'?` | +| Action | `mlc mrak-tmp cache` | `Did you mean 'mark-tmp'?` | +| Target | `mlc run scrip` | `Did you mean 'script'?` | +| Target | `mlc find cach` | `Did you mean 'cache'?` | +| Target | `mlc pull rpo` | `Did you mean 'repo'?` | + +## Implementation + +The feature is implemented in [`mlc/typo_mixin.py`](../mlc/typo_mixin.py) as a +Python mixin class — `TypoMixin` — that can be mixed into any +`argparse.ArgumentParser` subclass. + +```python +class TypoMixin: + def suggest(self, word, candidates) -> list[str]: ... + def error(self, message) -> None: ... +``` + +`TypoMixin` overrides `ArgumentParser.error()` to: + +1. Call `self.print_usage(sys.stderr)` (identical to the standard behaviour). +2. Parse the mistyped value and the valid-choices list from argparse's error + text using a regex. +3. Run `difflib.get_close_matches()` (Python stdlib, no extra dependencies) + with a similarity cutoff of `0.6`. +4. If one or more matches are found, write the hint line to stderr. +5. Call `self.exit(2, …)` with the original error message — identical to the + standard behaviour. + +In `main.py`, a concrete `TypoArgumentParser` class is assembled: + +```python +class TypoArgumentParser(TypoMixin, argparse.ArgumentParser): + pass +``` + +Both `build_pre_parser()` and `build_parser()` return `TypoArgumentParser` +instances. Subparsers created via `add_subparsers()` automatically inherit the +class (argparse uses `type(self)` as the default `parser_class`), so target +typos inside a valid command are also caught. + +## Tuning + +Two class-level attributes control the matching behaviour; override them on +`TypoArgumentParser` if needed: + +| Attribute | Default | Meaning | +|---|---|---| +| `_TYPO_CUTOFF` | `0.6` | Minimum similarity ratio (0–1). Raise to require a closer match. | +| `_TYPO_MAX_SUGGESTIONS` | `3` | Maximum number of alternatives displayed. | + +## Testing + +Tests live in [`tests/test_typo_mixin.py`](../tests/test_typo_mixin.py) and are +split into two suites: + +- **`TypoMixinSuggestTests`** — unit tests for `TypoMixin.suggest()` covering + single matches, multiple matches, edge cases (empty input, empty candidates, + exact match, transposed characters). + +- **`TypoMixinCliTests`** — subprocess integration tests that invoke + `python -m mlc.main ` and assert that the correct hint + appears in stderr with exit code 2. + +Run locally with: + +```bash +pip install -e . +python -m unittest tests/test_typo_mixin.py -v +``` + +Or via the full test suite: + +```bash +python -m unittest discover -s tests -p "test_*.py" -v +``` diff --git a/mlc/main.py b/mlc/main.py index 0250ad303..fb90fbcf2 100644 --- a/mlc/main.py +++ b/mlc/main.py @@ -13,12 +13,17 @@ from .cache_action import CacheAction from .cfg_action import CfgAction from .experiment_action import ExperimentAction +from .typo_mixin import TypoMixin from .item import Item from .action_factory import get_action from .logger import logger, logging +class TypoArgumentParser(TypoMixin, argparse.ArgumentParser): + """ArgumentParser that suggests close matches when the user mistype a command or target.""" + + class Automation: action_object = None automation_type = None @@ -330,7 +335,7 @@ def convert_hyphen_to_underscore_in_args(): def build_pre_parser(): - pre_parser = argparse.ArgumentParser(add_help=False) + pre_parser = TypoArgumentParser(add_help=False) pre_parser.add_argument( "action", nargs="?", @@ -352,7 +357,7 @@ def build_pre_parser(): def build_parser(pre_args): - parser = argparse.ArgumentParser( + parser = TypoArgumentParser( prog="mlc", description="Manage repos, scripts, and caches.", add_help=False) diff --git a/mlc/typo_mixin.py b/mlc/typo_mixin.py new file mode 100644 index 000000000..186113105 --- /dev/null +++ b/mlc/typo_mixin.py @@ -0,0 +1,111 @@ +"""TypoMixin: argparse error mixin that suggests near-matches on invalid choice errors. + +When a user types an unrecognised action or target, argparse normally exits +with a bare "invalid choice" message. TypoMixin intercepts that error and +inserts a "Did you mean …?" hint before the standard error line, using +difflib.get_close_matches (stdlib, no extra dependencies). + +Usage:: + + class TypoArgumentParser(TypoMixin, argparse.ArgumentParser): + pass + + parser = TypoArgumentParser(prog="mlc", ...) + # Subparsers created from this parser inherit the class automatically + # because argparse uses type(self) as the default parser_class. +""" + +import difflib +import re +import sys + + +class TypoMixin: + """Mixin for argparse.ArgumentParser that adds 'Did you mean?' suggestions. + + Drop this mixin to the left of argparse.ArgumentParser in your class MRO. + It overrides only error() — no other behaviour changes. + + Attributes + ---------- + _TYPO_CUTOFF : float + Minimum SequenceMatcher similarity ratio (0–1) for a candidate to be + shown. Defaults to 0.6 (same as difflib.get_close_matches default). + _TYPO_MAX_SUGGESTIONS : int + Maximum number of alternatives displayed. + """ + + _TYPO_CUTOFF: float = 0.6 + _TYPO_MAX_SUGGESTIONS: int = 3 + + # Matches argparse's invalid-choice message in Python 3.8 – 3.14: + # "invalid choice: 'rune' (choose from 'run', 'pull', 'test')" + # The value is repr()'d so it's surrounded by single quotes. + _INVALID_CHOICE_RE = re.compile( + r"invalid choice: '?([^'()\s]+)'? \(choose from ([^)]+)\)" + ) + + # ------------------------------------------------------------------ # + # Public helpers (tested directly) # + # ------------------------------------------------------------------ # + + def suggest(self, word: str, candidates) -> list: + """Return the closest matches for *word* from *candidates*. + + Thin wrapper around difflib.get_close_matches so callers and tests + can access the suggestion logic without triggering sys.exit. + + Parameters + ---------- + word: + The mistyped string entered by the user. + candidates: + Iterable of valid strings to compare against. + + Returns + ------- + list[str] + Up to _TYPO_MAX_SUGGESTIONS matches ordered by similarity, + or an empty list when no match exceeds _TYPO_CUTOFF. + """ + return difflib.get_close_matches( + word, + candidates, + n=self._TYPO_MAX_SUGGESTIONS, + cutoff=self._TYPO_CUTOFF, + ) + + # ------------------------------------------------------------------ # + # argparse.ArgumentParser override # + # ------------------------------------------------------------------ # + + def error(self, message: str) -> None: + """Print usage, an optional 'Did you mean?' hint, then the standard error. + + The output order mirrors standard argparse except for the hint line + injected between the usage block and the error message: + + usage: mlc [-h] {run,pull,...} ... + + Did you mean 'run'? + + mlc: error: argument command: invalid choice: 'rune' (…) + """ + self.print_usage(sys.stderr) + + m = self._INVALID_CHOICE_RE.search(message) + if m: + invalid = m.group(1) + # Choices are repr()'d in the error text: "'run', 'pull', ..." + raw_choices = [c.strip().strip("'\"") for c in m.group(2).split(",")] + suggestions = self.suggest(invalid, raw_choices) + if suggestions: + if len(suggestions) == 1: + hint = f"Did you mean '{suggestions[0]}'?" + else: + quoted = ", ".join(f"'{s}'" for s in suggestions) + hint = f"Did you mean one of: {quoted}?" + sys.stderr.write(f"\n{hint}\n\n") + + args = {"prog": self.prog, "message": message} + self.exit(2, "%(prog)s: error: %(message)s\n" % args) diff --git a/tests/test_typo_mixin.py b/tests/test_typo_mixin.py new file mode 100644 index 000000000..3329ddc43 --- /dev/null +++ b/tests/test_typo_mixin.py @@ -0,0 +1,219 @@ +"""Tests for TypoMixin — the 'Did you mean?' suggestion layer on argparse errors. + +Test structure +-------------- +TypoMixinSuggestTests + Unit tests for TypoMixin.suggest() in isolation (no subprocess needed). + +TypoMixinCliTests + Integration tests that run mlc as a subprocess and assert the hint text + appears in stderr. These tests follow the same subprocess pattern used + by test_cache_mark_tmp.py. +""" + +import argparse +import subprocess +import sys +import os +import unittest + +from mlc.typo_mixin import TypoMixin + +REPO_ROOT = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) + +# Minimal concrete class for unit-testing suggest() without a full CLI. +class _TestParser(TypoMixin, argparse.ArgumentParser): + pass + + +def _run_mlc(*args): + """Invoke mlc as a subprocess and return the CompletedProcess.""" + env = os.environ.copy() + existing = env.get("PYTHONPATH", "") + env["PYTHONPATH"] = REPO_ROOT if not existing else REPO_ROOT + os.pathsep + existing + return subprocess.run( + [sys.executable, "-m", "mlc.main", *args], + capture_output=True, + text=True, + check=False, + env=env, + ) + + +# --------------------------------------------------------------------------- # +# Unit tests — suggest() # +# --------------------------------------------------------------------------- # + +class TypoMixinSuggestTests(unittest.TestCase): + + def setUp(self): + self.parser = _TestParser(prog="mlc") + + # ---- single close match ------------------------------------------------ + + def test_suggests_run_for_rune(self): + candidates = ["run", "pull", "test", "add", "find", "rm"] + result = self.parser.suggest("rune", candidates) + self.assertEqual(result, ["run"]) + + def test_suggests_script_for_scrip(self): + candidates = ["script", "cache", "repo", "repos"] + result = self.parser.suggest("scrip", candidates) + self.assertEqual(result, ["script"]) + + def test_suggests_cache_for_cach(self): + candidates = ["script", "cache", "repo"] + result = self.parser.suggest("cach", candidates) + self.assertEqual(result, ["cache"]) + + def test_suggests_find_for_fnd(self): + candidates = ["run", "pull", "find", "search", "rm", "add"] + result = self.parser.suggest("fnd", candidates) + self.assertEqual(result, ["find"]) + + def test_suggests_pull_for_pul(self): + candidates = ["run", "pull", "test", "add"] + result = self.parser.suggest("pul", candidates) + self.assertEqual(result, ["pull"]) + + def test_suggests_search_for_searche(self): + candidates = ["run", "find", "search", "rm"] + result = self.parser.suggest("searche", candidates) + self.assertEqual(result, ["search"]) + + def test_suggests_repo_for_rpo(self): + candidates = ["script", "cache", "repo", "repos"] + result = self.parser.suggest("rpo", candidates) + # 'repo' and 'repos' are both close; at least one must be present + self.assertTrue(len(result) >= 1) + self.assertIn("repo", result) + + # ---- multiple suggestions ---------------------------------------------- + + def test_returns_multiple_when_several_close(self): + # 'rm' is very similar to 'rm'; 'cp' and 'mv' less so — just check + # the result is a list. + candidates = ["run", "rm", "cp", "mv", "add"] + result = self.parser.suggest("rm", candidates) + # exact match should be first + self.assertIn("rm", result) + + # ---- no match --------------------------------------------------------- + + def test_returns_empty_for_completely_different_word(self): + candidates = ["run", "pull", "test", "add", "find"] + result = self.parser.suggest("xyzzy", candidates) + self.assertEqual(result, []) + + def test_returns_empty_for_empty_string(self): + candidates = ["run", "pull", "test"] + result = self.parser.suggest("", candidates) + self.assertEqual(result, []) + + def test_returns_empty_for_empty_candidates(self): + result = self.parser.suggest("run", []) + self.assertEqual(result, []) + + # ---- cutoff & max ------------------------------------------------------- + + def test_respects_max_suggestions(self): + # 'scr' is close to 'script', 'search', 'scratch' — at most 3 returned + candidates = ["script", "search", "scratch", "screen", "run"] + result = self.parser.suggest("scr", candidates) + self.assertLessEqual(len(result), self.parser._TYPO_MAX_SUGGESTIONS) + + def test_exact_match_is_included(self): + candidates = ["run", "pull", "test"] + result = self.parser.suggest("run", candidates) + self.assertIn("run", result) + + def test_one_char_transposition(self): + # 'mrak-tmp' → 'mark-tmp' + candidates = ["run", "mark-tmp", "prune", "list"] + result = self.parser.suggest("mrak-tmp", candidates) + self.assertIn("mark-tmp", result) + + +# --------------------------------------------------------------------------- # +# Integration tests — CLI output # +# --------------------------------------------------------------------------- # + +class TypoMixinCliTests(unittest.TestCase): + """Run mlc as a subprocess and verify the hint appears in stderr.""" + + # ---- action-level typos ------------------------------------------------ + + def test_cli_suggests_run_for_rune(self): + result = _run_mlc("rune", "script") + self.assertEqual(result.returncode, 2) + # 'rune' is close to both 'run' and 'prune'; we just verify a hint + # containing 'run' is shown (single or multi-suggestion form). + self.assertIn("Did you mean", result.stderr) + self.assertIn("run", result.stderr) + + def test_cli_suggests_pull_for_pul(self): + result = _run_mlc("pul", "repo") + self.assertEqual(result.returncode, 2) + self.assertIn("Did you mean 'pull'?", result.stderr) + + def test_cli_suggests_find_for_fidn(self): + result = _run_mlc("fidn", "script") + self.assertEqual(result.returncode, 2) + self.assertIn("Did you mean 'find'?", result.stderr) + + def test_cli_suggests_search_for_serach(self): + result = _run_mlc("serach", "cache") + self.assertEqual(result.returncode, 2) + self.assertIn("Did you mean 'search'?", result.stderr) + + def test_cli_suggests_list_for_lst(self): + result = _run_mlc("lst", "cache") + self.assertEqual(result.returncode, 2) + self.assertIn("Did you mean 'list'?", result.stderr) + + def test_cli_suggests_mark_tmp_for_marktmp(self): + result = _run_mlc("mark_tmp", "cache") + self.assertEqual(result.returncode, 2) + # 'mark_tmp' vs 'mark-tmp' — close enough + self.assertIn("Did you mean", result.stderr) + + # ---- target-level typos ------------------------------------------------ + + def test_cli_suggests_script_for_scrip(self): + result = _run_mlc("run", "scrip") + self.assertEqual(result.returncode, 2) + self.assertIn("Did you mean 'script'?", result.stderr) + + def test_cli_suggests_cache_for_cach(self): + result = _run_mlc("find", "cach") + self.assertEqual(result.returncode, 2) + self.assertIn("Did you mean 'cache'?", result.stderr) + + def test_cli_suggests_repo_for_rpo(self): + result = _run_mlc("pull", "rpo") + self.assertEqual(result.returncode, 2) + self.assertIn("Did you mean", result.stderr) + self.assertIn("repo", result.stderr) + + # ---- no suggestion for totally wrong input ----------------------------- + + def test_cli_no_suggestion_for_garbage_command(self): + result = _run_mlc("xyzzy123", "script") + self.assertEqual(result.returncode, 2) + # Should still fail but not print a Did you mean hint + self.assertNotIn("Did you mean", result.stderr) + + # ---- error message still present --------------------------------------- + + def test_cli_still_shows_error_message(self): + result = _run_mlc("rune", "script") + self.assertIn("error:", result.stderr) + self.assertIn("rune", result.stderr) + + def test_cli_still_shows_usage(self): + result = _run_mlc("rune", "script") + self.assertIn("usage:", result.stderr) + + +if __name__ == "__main__": + unittest.main()