Skip to content

Makefile依存を廃止しPython CLI化・LLMs Provider構造化#2

Merged
uzulla merged 27 commits intomainfrom
python-cli-llm-provider
Apr 12, 2025
Merged

Makefile依存を廃止しPython CLI化・LLMs Provider構造化#2
uzulla merged 27 commits intomainfrom
python-cli-llm-provider

Conversation

@uzulla
Copy link
Copy Markdown
Owner

@uzulla uzulla commented Apr 12, 2025

概要

  • bin/myder runサブコマンドに --nomount および --force-yes オプションを追加
    • これらはCLIオプション・環境変数(NOMOUNT, FORCE_YES)両対応、CLIオプションが優先
  • help/descriptionに環境変数・オプションの説明を明記
  • デフォルトモデル名を openrouter/google/gemini-2.5-pro-exp-03-25:free に統一
  • runサブコマンドはmainブランチMakefile runターゲット相当のdocker runラッパー

主な修正内容

  • bin/myder: runサブコマンドのCLIオプション追加(--nomount, --force-yes)、help改善、デフォルトモデル名修正
  • src/myder_core.py: Provider import小文字対応、不要定数削除
  • .gitignore: pycache/除外
  • sample_provider関連の古いテストは未整理(今後対応予定)

使い方例

bin/myder run --model openrouter/google/gemini-2.5-pro-exp-03-25:free --nomount --force-yes
NOMOUNT=1 FORCE_YES=1 bin/myder run
  • .envファイルがあれば自動で--env-fileに渡されます

確認してほしい点

  • runサブコマンドのCLIオプション・環境変数の優先順位
  • help/descriptionの記載内容
  • デフォルトモデル名の統一
  • 既存運用・要件との齟齬がないか

重要点

runサブコマンドのCLIオプション

  • --nomount, --force-yesはCLIオプション・環境変数両対応、CLIオプションが優先
  • help/descriptionに明記

デフォルト値・運用

  • デフォルトモデル名はopenrouter/google/gemini-2.5-pro-exp-03-25:free
  • .env, NOMOUNT, FORCE_YES等の運用も明記

備考

  • Provider直実行CLI分離・古いテスト整理は今後対応予定

uzulla+oh added 23 commits April 12, 2025 16:59
Why:
Makefileベースの運用が複雑で拡張性・保守性に課題があったため、Python CLI化し、Providerを柔軟に追加できる構造にする。

What/How:
- myder.py(Python CLI)新規作成
- provider/ ディレクトリとサンプルProvider実装
- README.mdに新CLIの使い方とMakefile廃止予定を追記
Why:
Makefile依存を廃止し、今後はPython CLI(myder.py)運用へ移行するため。

What/How:
- Makefileをリポジトリから削除
Why:
Makefileは既に削除されており、「廃止予定」ではなく「廃止済み」とするのが正確なため。

What/How:
- README冒頭の案内文を現状に合わせて修正
Why:
テスト容易性・保守性向上のため、CLIとProvider管理/実行ロジックを分離し、ユニットテストを追加。

What/How:
- myder_core.py新設(Provider管理・実行ロジック集約)
- myder.pyはCLI引数処理のみ担当にリファクタ
- pytest形式のユニットテスト(tests/test_myder_core.py)追加
Pythonのバイトコードキャッシュ(__pycache__)をバージョン管理対象外とするため、.gitignoreの末尾に追加
開発・CI環境で不要なファイルが混入しないようにするため
.gitignoreに追加済みのため、今後はバージョン管理対象外とする
bin/myder, src/myder_core.py等の新構成に合わせて旧ファイルを削除
テスト・READMEも現状に即して修正
ファイル修正時に生成されたバックアップ(.orig)ファイルを不要のため削除
Provider基底クラス(base.py)新設、OpenRouterProvider実装例追加
myder_core.pyをAPI Key必須設計にリファクタ
OpenRouterProviderのユニットテスト追加
CLIエントリポイントとして直接実行できるように修正
DEFAULT_PROVIDER=openrouter, DEFAULT_MODEL=gemini-2.5をmyder_core.pyに定義し、bin/myderで省略時に利用
runサブコマンドでprovider名に応じたAPI KEYをCLI引数・環境変数・.envから取得しrun_providerに渡すよう修正
to_pascal_case関数を追加し、openrouter→OpenRouterProvider等の命名に対応
OpenRouterなどキャメルケース指定で動作する設計に変更
DEFAULT_PROVIDERをopenrouter→OpenRouterに変更し、キャメルケース指定に統一
--provider OpenRouter でも provider/openrouter.py をimportし、OpenRouterProviderクラスを参照できるように
Providerのrun()が返す推論結果等が標準出力に表示されるように
mainブランチMakefileのrunターゲット相当のdocker runコマンドをPythonで再現
--model未指定時も常にopenrouter/google/gemini-2.5-pro-exp-03-25:freeを利用するよう統一
現状のCLI設計では参照されていないため不要
ユーザーが利用可能な環境変数オプションをhelp/descriptionに明記
NOMOUNT, FORCE_YESは環境変数だけでなくCLIオプションでも指定可能にし、オプションが優先されるように修正
@uzulla uzulla marked this pull request as ready for review April 12, 2025 19:25
@uzulla uzulla requested a review from Copilot April 12, 2025 19:25
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot reviewed 9 out of 10 changed files in this pull request and generated 2 comments.

Files not reviewed (1)
  • Makefile: Language not supported

Comment thread tests/test_myder_core.py Outdated
def test_run_provider(capsys):
myder_core.run_provider("sample_provider", model="test-model")
captured = capsys.readouterr()
assert "SampleProvider: running with model=test-model" in captured.outdef test_openrouter_provider_properties():
Copy link

Copilot AI Apr 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There appears to be a syntax error where a newline is missing between the assert statement and the subsequent function definition. Please insert a newline to separate these statements.

Suggested change
assert "SampleProvider: running with model=test-model" in captured.outdef test_openrouter_provider_properties():
assert "SampleProvider: running with model=test-model" in captured.out
def test_openrouter_provider_properties():

Copilot uses AI. Check for mistakes.
Comment thread src/myder_core.py Outdated
Comment on lines +17 to +20
def load_provider(name, api_key):
module = importlib.import_module(f"provider.{name.lower()}")
class_name = name + "Provider"
provider_class = getattr(module, class_name)
Copy link

Copilot AI Apr 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The constructed class name does not match the actual provider class names (e.g., for 'sample_provider', the class is defined as 'Provider', not 'sample_providerProvider'). Consider revising the naming strategy or using a mapping to correctly retrieve the provider class.

Suggested change
def load_provider(name, api_key):
module = importlib.import_module(f"provider.{name.lower()}")
class_name = name + "Provider"
provider_class = getattr(module, class_name)
import inspect
def load_provider(name, api_key):
module = importlib.import_module(f"provider.{name.lower()}")
provider_class = None
for _, cls in inspect.getmembers(module, inspect.isclass):
if cls.__module__ == module.__name__:
provider_class = cls
break
if provider_class is None:
raise ImportError(f"No valid provider class found in module 'provider.{name.lower()}'")

Copilot uses AI. Check for mistakes.
Why: PRレビューで指摘された構文エラーとProviderクラス名取得の不整合を解消するため
What: テストのassert文とdefの間に改行を追加、inspectによるProviderクラス自動検出に修正
How: Copilotのサジェスト通りに修正
@uzulla uzulla requested a review from Copilot April 12, 2025 19:42
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot reviewed 9 out of 10 changed files in this pull request and generated 3 comments.

Files not reviewed (1)
  • Makefile: Language not supported

Comment thread tests/test_myder_core.py Outdated
assert "sample_provider" in providers

def test_load_provider():
provider = myder_core.load_provider("sample_provider")
Copy link

Copilot AI Apr 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The load_provider function requires an api_key argument, but none is provided here. Consider passing a dummy value (e.g., 'dummy-key') for consistency with the provider initialization.

Suggested change
provider = myder_core.load_provider("sample_provider")
provider = myder_core.load_provider("sample_provider", api_key="dummy-key")

Copilot uses AI. Check for mistakes.
Comment thread tests/test_myder_core.py Outdated
Comment on lines +17 to +18
myder_core.run_provider("sample_provider", model="test-model")
captured = capsys.readouterr()
Copy link

Copilot AI Apr 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The run_provider function requires an api_key argument as its second parameter. Update the call to include a dummy api_key, such as 'dummy-key'.

Suggested change
myder_core.run_provider("sample_provider", model="test-model")
captured = capsys.readouterr()
myder_core.run_provider("sample_provider", api_key="dummy-key", model="test-model")

Copilot uses AI. Check for mistakes.
Comment thread src/myder_core.py Outdated
Comment on lines +6 to +7
import os

Copy link

Copilot AI Apr 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] There is a duplicate import of 'os'. Removing the redundant import at line 6 will clean up the code.

Suggested change
import os

Copilot uses AI. Check for mistakes.
Why: PRレビューでapi_key引数漏れとos重複importを指摘されたため
What: test_load_provider/test_run_providerにapi_key="dummy-key"追加、src/myder_core.pyの重複import削除
How: Copilotのサジェスト通り修正
@uzulla uzulla requested a review from Copilot April 12, 2025 19:52
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot reviewed 9 out of 10 changed files in this pull request and generated 1 comment.

Files not reviewed (1)
  • Makefile: Language not supported

Comment thread src/provider/sample_provider.py Outdated
@@ -0,0 +1,3 @@
class Provider:
def run(self, model=None):
print(f"SampleProvider: running with model={model}") No newline at end of file
Copy link

Copilot AI Apr 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The 'run' method in the sample provider prints its output instead of returning it, which is inconsistent with OpenRouterProvider's implementation. Consider standardizing the behavior across providers by either always returning a string or handling output uniformly.

Suggested change
print(f"SampleProvider: running with model={model}")
return f"SampleProvider: running with model={model}"

Copilot uses AI. Check for mistakes.
Why: PRレビューで出力方法の不統一を指摘されたため
What: printからreturnに修正し、OpenRouterProviderと統一
How: Copilotのサジェスト通り修正
@uzulla uzulla requested a review from Copilot April 12, 2025 19:55
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot reviewed 9 out of 10 changed files in this pull request and generated no comments.

Files not reviewed (1)
  • Makefile: Language not supported
Comments suppressed due to low confidence (1)

src/provider/sample_provider.py:1

  • [nitpick] Consider renaming the class 'Provider' to 'SampleProvider' to avoid ambiguity with other provider classes and improve clarity.
class Provider:

@uzulla uzulla requested a review from Copilot April 12, 2025 19:57
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot reviewed 9 out of 10 changed files in this pull request and generated no comments.

Files not reviewed (1)
  • Makefile: Language not supported
Comments suppressed due to low confidence (1)

src/provider/sample_provider.py:1

  • [nitpick] Consider renaming the class 'Provider' to 'SampleProvider' for clarity and consistency with other provider classes like 'OpenRouterProvider'.
class Provider:

Why: Providerという名前が曖昧で、親クラスであることを明確にするため
What: class Provider → class BaseProvider に変更
How: nitpick指摘に従い修正
@uzulla uzulla requested a review from Copilot April 12, 2025 19:59
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot reviewed 9 out of 10 changed files in this pull request and generated no comments.

Files not reviewed (1)
  • Makefile: Language not supported

@uzulla uzulla merged commit a0d47a4 into main Apr 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants