From fea7b3fd42ec62ec4a0c0d2caeb56ea7810d442b Mon Sep 17 00:00:00 2001 From: Pablo Ilardi Date: Sun, 4 Jan 2026 12:12:27 -0500 Subject: [PATCH] refactor: Simplify CLI command signatures and improve configuration handling - Remove redundant CLI options from command functions, centralizing configuration via Runtime settings. - Add `Runtime.set_value` method to allow programmatic setting of configuration values. - Update README examples to reflect corrected command usage. - Adjust test cases to align with the new configuration approach and command signatures. - Reorder CLI arguments in integration tests for consistency and correctness. --- README.md | 4 +- src/git_llm_utils/app.py | 147 ++++++++------------------------------- tests/test_generate.py | 42 +++++------ tests/test_server.py | 133 ++++++++++++++++++++--------------- 4 files changed, 127 insertions(+), 199 deletions(-) diff --git a/README.md b/README.md index 29bb31c..a2e21cc 100644 --- a/README.md +++ b/README.md @@ -101,11 +101,11 @@ to change the setting, where *setting* is any of the available [settings](#setti By default the system uses [`qwen3-coder:480b-cloud`](https://ollama.com/library/qwen3-coder:480b-cloud), however you can try other models such as [`nemotron-3-nano:30b-cloud`](https://ollama.com/library/nemotron-3-nano). ie: ```bash -ollama pull nemotron-3-nano:30b-cloud && git-llm-utils commit --model ollama/nemotron-3-nano:30b-cloud +ollama pull nemotron-3-nano:30b-cloud && git-llm-utils --model ollama/nemotron-3-nano:30b-cloud commit ``` Local models are also supported however the quality of the message will drasticall vary based on the size of it (even worse for non-thinking models), ie: ```bash -ollama pull deepseek-coder:6.7b && git-llm-utils commit --model ollama/deepseek-coder:6.7b --reasoning none +ollama pull deepseek-coder:6.7b && git-llm-utils --model ollama/deepseek-coder:6.7b --reasoning none commit ``` ## References: diff --git a/src/git_llm_utils/app.py b/src/git_llm_utils/app.py index e68ac42..16112b3 100644 --- a/src/git_llm_utils/app.py +++ b/src/git_llm_utils/app.py @@ -157,6 +157,11 @@ def set_config( else: unset_config(s.key, scope=scope, repository=Runtime.repository) + @staticmethod + def set_value(setting: str, given: Any = None): + if setting in Runtime.settings: + return Runtime.settings[setting][0].set_value(given) + @staticmethod def get_value(setting: str, given: Any = None) -> Optional[Any]: if setting in Runtime.settings: @@ -252,10 +257,9 @@ def __new__(cls, loader: SettingLoader, option: typer.models.OptionInfo): @app.command(help="Reads the respository description") -def description( - description_file: str = Setting.DESCRIPTION_FILE.option, # type: ignore -) -> Optional[str]: - print(read_file(file_path=Path(Runtime.repository or ".", description_file))) +def description() -> Optional[str]: + description_file = Runtime.get_value(Setting.DESCRIPTION_FILE.value) # type: ignore + print(read_file(file_path=Path(Runtime.repository or ".", description_file))) # type: ignore @app.command( @@ -263,30 +267,12 @@ def description( ) def command( args: list[str], - with_emojis: bool | None = Setting.EMOJIS.option, # type: ignore with_comments: bool | None = Setting.COMMENTS.option, # type: ignore - model: str | None = Setting.MODEL.option, # type: ignore - reasoning: str | None = Setting.MODEL_REASONING.option, # type: ignore - max_input_tokens: int | None = Setting.MAX_INPUT_TOKENS.option, # type: ignore - max_output_tokens: int | None = Setting.MAX_OUTPUT_TOKENS.option, # type: ignore - api_key: str | None | None = Setting.API_KEY.option, # type: ignore - api_url: str | None | None = Setting.API_URL.option, # type: ignore - description_file: str | None = Setting.DESCRIPTION_FILE.option, # type: ignore - tools: bool | None = Setting.TOOLS.option, # type: ignore editor: str | None = Setting.EDITOR.option, # type: ignore ): output = StringIO() generated = _message( - with_emojis=with_emojis, with_comments=with_comments, - model=model, - reasoning=reasoning, - max_input_tokens=max_input_tokens, - max_output_tokens=max_output_tokens, - api_key=api_key, - api_url=api_url, - description_file=description_file, - tools=tools, output=output, ) value = output.getvalue() @@ -310,57 +296,20 @@ def command( help=f"commit the current change set using the status message using `{' '.join(COMMIT_COMMAND)}`" ) def commit( - with_emojis: bool | None = Setting.EMOJIS.option, # type: ignore with_comments: bool | None = Setting.COMMENTS.option, # type: ignore - model: str | None = Setting.MODEL.option, # type: ignore - reasoning: str | None = Setting.MODEL_REASONING.option, # type: ignore - max_input_tokens: int | None = Setting.MAX_INPUT_TOKENS.option, # type: ignore - max_output_tokens: int | None = Setting.MAX_OUTPUT_TOKENS.option, # type: ignore - api_key: str | None | None = Setting.API_KEY.option, # type: ignore - api_url: str | None | None = Setting.API_URL.option, # type: ignore - description_file: str | None = Setting.DESCRIPTION_FILE.option, # type: ignore - tools: bool | None = Setting.TOOLS.option, # type: ignore editor: str | None = Setting.EDITOR.option, # type: ignore ): command( COMMIT_COMMAND, - with_emojis=with_emojis, with_comments=with_comments, - model=model, - reasoning=reasoning, - max_input_tokens=max_input_tokens, - max_output_tokens=max_output_tokens, - api_key=api_key, - api_url=api_url, - description_file=description_file, - tools=tools, editor=editor, ) @app.command(help="Generates a status message based on the git staged changes") -def status( - with_emojis: bool | None = Setting.EMOJIS.option, # type: ignore - model: str | None = Setting.MODEL.option, # type: ignore - reasoning: str | None = Setting.MODEL_REASONING.option, # type: ignore - max_input_tokens: int | None = Setting.MAX_INPUT_TOKENS.option, # type: ignore - max_output_tokens: int | None = Setting.MAX_OUTPUT_TOKENS.option, # type: ignore - api_key: str | None | None = Setting.API_KEY.option, # type: ignore - api_url: str | None | None = Setting.API_URL.option, # type: ignore - description_file: str | None = Setting.DESCRIPTION_FILE.option, # type: ignore - tools: bool | None = Setting.TOOLS.option, # type: ignore -): +def status(): res = _message( - with_emojis=with_emojis, with_comments=False, - model=model, - reasoning=reasoning, - max_input_tokens=max_input_tokens, - max_output_tokens=max_output_tokens, - api_key=api_key, - api_url=api_url, - description_file=description_file, - tools=tools, output=sys.stdout, ) if res: @@ -368,28 +317,9 @@ def status( @app.command(help="Verifies the configuration allows accessing the llm") -def verify( - with_emojis: bool | None = Setting.EMOJIS.option, # type: ignore - model: str | None = Setting.MODEL.option, # type: ignore - reasoning: str | None = Setting.MODEL_REASONING.option, # type: ignore - max_input_tokens: int | None = Setting.MAX_INPUT_TOKENS.option, # type: ignore - max_output_tokens: int | None = Setting.MAX_OUTPUT_TOKENS.option, # type: ignore - api_key: str | None | None = Setting.API_KEY.option, # type: ignore - api_url: str | None | None = Setting.API_URL.option, # type: ignore - description_file: str | None = Setting.DESCRIPTION_FILE.option, # type: ignore - tools: bool | None = Setting.TOOLS.option, # type: ignore -): +def verify(): res = _message( - with_emojis=with_emojis, with_comments=False, - model=model, - reasoning=reasoning, - max_input_tokens=max_input_tokens, - max_output_tokens=max_output_tokens, - api_key=api_key, - api_url=api_url, - description_file=description_file, - tools=tools, output=sys.stdout, get_changes=lambda _: """diff --git a/Readme.md b/Readme.md new file mode 100644 @@ -410,16 +340,7 @@ def verify( help=f"Generates a commit message based on the git staged changes for the {MESSAGE_HOOK} hook" ) def generate( - with_emojis: bool | None = Setting.EMOJIS.option, # type: ignore with_comments: bool | None = Setting.COMMENTS.option, # type: ignore - model: str | None = Setting.MODEL.option, # type: ignore - reasoning: str | None = Setting.MODEL_REASONING.option, # type: ignore - max_input_tokens: int | None = Setting.MAX_INPUT_TOKENS.option, # type: ignore - max_output_tokens: int | None = Setting.MAX_OUTPUT_TOKENS.option, # type: ignore - api_key: str | None = Setting.API_KEY.option, # type: ignore - api_url: str | None = Setting.API_URL.option, # type: ignore - description_file: str | None = Setting.DESCRIPTION_FILE.option, # type: ignore - tools: bool | None = Setting.TOOLS.option, # type: ignore manual: bool | None = Setting.MANUAL.option, # type: ignore manual_override: bool = typer.Option( None, @@ -436,31 +357,13 @@ def generate( return False return _message( - with_emojis=with_emojis, with_comments=with_comments, - model=model, - reasoning=reasoning, - max_input_tokens=max_input_tokens, - max_output_tokens=max_output_tokens, - api_key=api_key, - api_url=api_url, - description_file=description_file, - tools=tools, output=output, ) def _message( - with_emojis: bool | None = Setting.EMOJIS.option, # type: ignore - with_comments: bool | None = Setting.COMMENTS.option, # type: ignore - model: str | None = Setting.MODEL.option, # type: ignore - reasoning: str | None = Setting.MODEL_REASONING.option, # type: ignore - max_input_tokens: int | None = Setting.MAX_INPUT_TOKENS.option, # type: ignore - max_output_tokens: int | None = Setting.MAX_OUTPUT_TOKENS.option, # type: ignore - api_key: str | None = Setting.API_KEY.option, # type: ignore - api_url: str | None = Setting.API_URL.option, # type: ignore - description_file: str | None = Setting.DESCRIPTION_FILE.option, # type: ignore - tools: bool | None = Setting.TOOLS.option, # type: ignore + with_comments: bool | None, output: TextIO = typer.Option( hidden=True, parser=lambda _: sys.stdout, default=sys.stdout ), @@ -471,20 +374,19 @@ def _message( print(NO_CHANGES_MESSAGE, file=output) return False + description_file = Runtime.get_value(Setting.DESCRIPTION_FILE.value) file_path = ( description_file and Path(Runtime.repository or ".", description_file) or None ) client = LLMClient( - use_emojis=Runtime.get_value(Setting.EMOJIS.value, with_emojis), # type: ignore - model_name=Runtime.get_value(Setting.MODEL.value, model), # type: ignore - model_reasoning=Runtime.get_value(Setting.MODEL_REASONING.value, reasoning), # type: ignore - max_tokens=Runtime.get_value(Setting.MAX_INPUT_TOKENS.value, max_input_tokens), # type: ignore - max_output_tokens=Runtime.get_value( - Setting.MAX_OUTPUT_TOKENS.value, max_output_tokens - ), # type: ignore - api_key=Runtime.get_value(Setting.API_KEY.value, api_key), - api_url=Runtime.get_value(Setting.API_URL.value, api_url), - use_tools=Runtime.get_value(Setting.TOOLS.value, tools) + use_emojis=Runtime.get_value(Setting.EMOJIS.value), # type: ignore + model_name=Runtime.get_value(Setting.MODEL.value), # type: ignore + model_reasoning=Runtime.get_value(Setting.MODEL_REASONING.value), + max_tokens=Runtime.get_value(Setting.MAX_INPUT_TOKENS.value), # type: ignore + max_output_tokens=Runtime.get_value(Setting.MAX_OUTPUT_TOKENS.value), # type: ignore + api_key=Runtime.get_value(Setting.API_KEY.value), + api_url=Runtime.get_value(Setting.API_URL.value), + use_tools=Runtime.get_value(Setting.TOOLS.value) and file_path is not None and file_path.exists(), # type: ignore respository_description=lambda: read_file(file_path), # type: ignore @@ -716,6 +618,15 @@ def _( allow_from_autoenv=True, envvar="GIT_LLM_REPO", ), + description_file: str | None = Setting.DESCRIPTION_FILE.option, # type: ignore + model: str | None = Setting.MODEL.option, # type: ignore + api_key: str | None = Setting.API_KEY.option, # type: ignore + api_url: str | None = Setting.API_URL.option, # type: ignore + tools: bool | None = Setting.TOOLS.option, # type: ignore + reasoning: str | None = Setting.MODEL_REASONING.option, # type: ignore + max_input_tokens: int | None = Setting.MAX_INPUT_TOKENS.option, # type: ignore + max_output_tokens: int | None = Setting.MAX_OUTPUT_TOKENS.option, # type: ignore + with_emojis: bool | None = Setting.EMOJIS.option, # type: ignore confirm: bool = typer.Option( help="Requests confirmation before changing a setting", callback=Runtime._set_confirm, diff --git a/tests/test_generate.py b/tests/test_generate.py index 9da4c2a..3fa3723 100644 --- a/tests/test_generate.py +++ b/tests/test_generate.py @@ -6,7 +6,7 @@ from types import SimpleNamespace from typing import Any -from git_llm_utils.app import generate, NO_CHANGES_MESSAGE +from git_llm_utils.app import generate, Runtime, Setting, NO_CHANGES_MESSAGE from git_llm_utils.llm import LLMClient @@ -32,12 +32,12 @@ def _mock(monkeypatch, changes: str | Any = None, message: str | Any = None): def test_generate_manual_does_nothing(monkeypatch): _mock(monkeypatch) res = StringIO() + Runtime.set_value(Setting.MAX_INPUT_TOKENS.value, 1000) + Runtime.set_value(Setting.MAX_OUTPUT_TOKENS.value, 1000) generate( manual=True, manual_override=False, output=res, - max_input_tokens=1000, - max_output_tokens=1000, ) res.seek(0) assert res.read() == "" @@ -46,12 +46,12 @@ def test_generate_manual_does_nothing(monkeypatch): def test_generate_with_no_change(monkeypatch): _mock(monkeypatch) res = StringIO() + Runtime.set_value(Setting.MAX_INPUT_TOKENS.value, 1000) + Runtime.set_value(Setting.MAX_OUTPUT_TOKENS.value, 1000) generate( manual=False, manual_override=True, output=res, - max_input_tokens=1000, - max_output_tokens=1000, ) res.seek(0) assert res.read() == f"{NO_CHANGES_MESSAGE}\n" @@ -59,19 +59,19 @@ def test_generate_with_no_change(monkeypatch): def test_generate_with_comments(monkeypatch): _mock(monkeypatch, "test", "test") + Runtime.set_value(Setting.MODEL.value, "test") + Runtime.set_value(Setting.API_KEY.value, "test") + Runtime.set_value(Setting.API_URL.value, "test") + Runtime.set_value(Setting.MODEL_REASONING.value, "high") + Runtime.set_value(Setting.MAX_INPUT_TOKENS.value, 1000) + Runtime.set_value(Setting.MAX_OUTPUT_TOKENS.value, 1000) + Runtime.set_value(Setting.EMOJIS.value, False) + Runtime.set_value(Setting.DESCRIPTION_FILE.value, None) res = StringIO() generate( manual=False, manual_override=True, with_comments=True, - description_file=None, - with_emojis=False, - model="test", - reasoning="high", - max_input_tokens=1000, - max_output_tokens=1000, - api_key="test", - api_url="test", output=res, ) res.seek(0) @@ -80,19 +80,19 @@ def test_generate_with_comments(monkeypatch): def test_generate_without_comments(monkeypatch): _mock(monkeypatch, "test", "test") + Runtime.set_value(Setting.MODEL.value, "test") + Runtime.set_value(Setting.API_KEY.value, "test") + Runtime.set_value(Setting.API_URL.value, "test") + Runtime.set_value(Setting.MODEL_REASONING.value, "high") + Runtime.set_value(Setting.MAX_INPUT_TOKENS.value, 1000) + Runtime.set_value(Setting.MAX_OUTPUT_TOKENS.value, 1000) + Runtime.set_value(Setting.EMOJIS.value, False) + Runtime.set_value(Setting.DESCRIPTION_FILE.value, None) res = StringIO() generate( manual=False, manual_override=True, with_comments=False, - description_file=None, - with_emojis=False, - model="test", - reasoning="high", - max_input_tokens=1000, - max_output_tokens=1000, - api_key="test", - api_url="test", output=res, ) res.seek(0) diff --git a/tests/test_server.py b/tests/test_server.py index c60fd73..0203ab0 100644 --- a/tests/test_server.py +++ b/tests/test_server.py @@ -152,14 +152,14 @@ def repository_2(tmp_path_factory: pytest.TempPathFactory): def test_verify(cmd: list[str], mock_server: str): verification = _read_file("tests/files/verification.out") args = cmd + [ - "verify", - "--api-url", - mock_server, + "--no-with-emojis", "--model", API_MODEL, + "--api-url", + mock_server, "--api-key", API_KEY_TOKEN, - "--no-with-emojis", + "verify", ] cli_status = execute_command(args) assert cli_status is not None @@ -169,31 +169,40 @@ def test_verify(cmd: list[str], mock_server: str): @pytest.mark.integration def test_status_with_no_emojis(cmd: list[str], repository: str, mock_server: str): status = _read_file("tests/files/status.out") - args = cmd + [ - "--debug", - "status", - "--api-url", - mock_server, - "--model", - API_MODEL, - "--api-key", - API_KEY_TOKEN, - "--no-with-emojis", - ] cli_status = execute_command( - args, + cmd + + [ + "--no-with-emojis", + "--debug", + "--model", + API_MODEL, + "--api-url", + mock_server, + "--api-key", + API_KEY_TOKEN, + "status", + ], cwd=repository, ) assert cli_status is not None assert status == cli_status.rstrip() execute_command( # update settings - cmd + ["set-config", "emojis", "--scope", "local", "--value", "False"], + cmd + ["set-config", "emojis", "--value", "False"], cwd=repository, ) - del args[-1] cli_status = execute_command( - args, + cmd + + [ + "--debug", + "--model", + API_MODEL, + "--api-url", + mock_server, + "--api-key", + API_KEY_TOKEN, + "status", + ], cwd=repository, ) assert cli_status is not None @@ -203,30 +212,38 @@ def test_status_with_no_emojis(cmd: list[str], repository: str, mock_server: str @pytest.mark.integration def test_status_with_emojis(cmd: list[str], repository: str, mock_server: str): status = _read_file("tests/files/status_with_emojis.out") - args = cmd + [ - "status", - "--api-url", - mock_server, - "--model", - API_MODEL, - "--api-key", - API_KEY_TOKEN, - "--with-emojis", - ] cli_status = execute_command( - args, + cmd + + [ + "--with-emojis", + "--model", + API_MODEL, + "--api-url", + mock_server, + "--api-key", + API_KEY_TOKEN, + "status", + ], cwd=repository, ) assert cli_status is not None assert status == cli_status.rstrip() execute_command( # update settings - cmd + ["set-config", "emojis", "--scope", "local", "--value", "True"], + cmd + ["set-config", "emojis", "--value", "True"], cwd=repository, ) - del args[-1] cli_status = execute_command( - args, + cmd + + [ + "--model", + API_MODEL, + "--api-url", + mock_server, + "--api-key", + API_KEY_TOKEN, + "status", + ], cwd=repository, ) assert cli_status is not None @@ -237,13 +254,13 @@ def test_status_with_emojis(cmd: list[str], repository: str, mock_server: str): def test_generate_with_no_comments(cmd: list[str], repository: str, mock_server: str): status = _read_file("tests/files/generate_with_no_comments.out") args = cmd + [ - "generate", - "--api-url", - mock_server, "--model", API_MODEL, + "--api-url", + mock_server, "--api-key", API_KEY_TOKEN, + "generate", "--no-manual", "--no-with-comments", ] @@ -271,13 +288,13 @@ def test_generate_with_no_comments(cmd: list[str], repository: str, mock_server: def test_generate_with_comments(cmd: list[str], repository: str, mock_server: str): status = _read_file("tests/files/generate_with_comments.out") args = cmd + [ - "generate", - "--api-url", - mock_server, "--model", API_MODEL, + "--api-url", + mock_server, "--api-key", API_KEY_TOKEN, + "generate", "--no-manual", "--with-comments", ] @@ -307,15 +324,15 @@ def test_generate_with_comments_and_repository( ): status = _read_file("tests/files/generate_with_comments.out") args = cmd + [ + "--model", + API_MODEL, "--repository", repository, - "generate", "--api-url", mock_server, - "--model", - API_MODEL, "--api-key", API_KEY_TOKEN, + "generate", "--no-manual", "--with-comments", ] @@ -552,13 +569,13 @@ def test_command(cmd: list[str], repository: str, mock_server: str): changeset = execute_command( cmd + [ - "command", - "--api-url", - mock_server, "--model", API_MODEL, + "--api-url", + mock_server, "--api-key", API_KEY_TOKEN, + "command", "--no-with-comments", "--", "git", @@ -585,15 +602,15 @@ def test_command_with_editor(cmd: list[str], repository: str, mock_server: str): changeset = execute_command( cmd + [ - "command", - "--editor", - "touch", - "--api-url", - mock_server, "--model", API_MODEL, + "--api-url", + mock_server, "--api-key", API_KEY_TOKEN, + "command", + "--editor", + "touch", "--", "git", "commit", @@ -613,13 +630,13 @@ def test_commit(cmd: list[str], repository: str, mock_server: str): changeset = execute_command( cmd + [ - "commit", - "--api-url", - mock_server, "--model", API_MODEL, + "--api-url", + mock_server, "--api-key", API_KEY_TOKEN, + "commit", "--no-with-comments", ], abort_on_error=True, @@ -639,15 +656,15 @@ def test_commit_with_editor(cmd: list[str], repository: str, mock_server: str): changeset = execute_command( cmd + [ - "commit", - "--editor", - "touch", - "--api-url", - mock_server, "--model", API_MODEL, + "--api-url", + mock_server, "--api-key", API_KEY_TOKEN, + "commit", + "--editor", + "touch", ], abort_on_error=True, valid_codes=[0, 256 + ErrorHandler.EMPTY_MESSAGE],