Refactor flashcard parsing - #27
Merged
Merged
Conversation
- Replace anthropic SDK with litellm for universal LLM support - Add support for 8 major providers: Anthropic, OpenAI, Google, Groq, Azure, Cohere, Together AI, Mistral - Update wizard to let users select provider before entering API key - Add AI_PROVIDER and AI_MODEL config options - Simplify AI client code from 647 to 628 lines - Maintain backwards compatibility with existing ANTHROPIC_API_KEY setups - Update README with all supported providers
- Update wizard to ask for MODEL (Claude 4, GPT-4o, etc.) not provider - Add 8 popular model choices with human-friendly names - Simplify README - remove exhaustive provider list - Update litellm to >=1.8.0 - Add DeepSeek V3 as model option - Remove custom model prompt (can use config if needed)
- Remove Gemini 1.5 Pro and Llama (old/niche) - Keep the big 4: Claude Sonnet 4, Claude Opus 4, GPT-4o, Gemini 2.0 Flash - Off-brand options: GPT-4o Mini, DeepSeek V3 - Reorder to put best models first
Top tier models: - Claude Sonnet 4 - Claude Opus 4 - GPT-5 - Gemini 3 Pro Preview Budget options: - GPT-4o - GPT-4o Mini - Gemini 2.5 Flash (not 2.0) - DeepSeek V3.1 (not V3)
Users can now do: oki config set model "Claude Sonnet 4" oki config set model "GPT-5" oki config set model "Gemini 3 Pro Preview" Instead of having to set ai_provider and ai_model separately with technical names. The config command automatically maps human names to provider + model.
…ps://github.com/ccmdi/obsidianki into claude/codebase-review-01KknsD8mSzzuNTdeo2RhVaa
- Replace AI_PROVIDER and AI_MODEL with single MODEL config - Users now only see human-friendly names like "Claude Sonnet 4" - Model mapping handled internally by FlashcardAI - Backwards compatible with old configs - Cleaner config output (just 'model: Claude Sonnet 4')
…SzzuNTdeo2RhVaa' into claude/codebase-review-01KknsD8mSzzuNTdeo2RhVaa
- Add mock for questionary.select() in test_setup.py to fix EOFError - Initialize model_choice=None in wizard.py to prevent UnboundLocalError - Only set MODEL in config if model_choice was set during setup - All 5 setup tests now pass (test_setup.py) - 179/180 total tests passing
The test_deck_list failure was caused by command modules holding stale references to service objects. When deck_cmd.py imports ANKI at the top level, it creates a reference before the mock_services fixture can replace it. Solution: Reload all modules that import from services after setting up mocks, so they pick up the mocked instances instead of the originals. - Reload deck_cmd, config_cmd, stats_cmd, schema_cmd, edit_mode - All 180 tests now pass
Added 11 tests to verify multi-provider LLM functionality: TestModelMap (6 tests): - Verify all 8 expected models exist in MODEL_MAP - Ensure each model has required fields (provider, model, key_name) - Validate Anthropic models use correct provider/API key - Validate OpenAI models use correct provider/API key - Validate Google models use correct provider/API key - Validate DeepSeek models use correct provider/API key TestFlashcardAIModelSelection (2 tests): - Verify FlashcardAI defaults to Claude Sonnet 4.5 - Verify FlashcardAI respects CONFIG.model for different providers TestModelConfiguration (2 tests): - Verify all MODEL_MAP keys are user-friendly names - Ensure no technical IDs like "claude-sonnet-4.5-20250514" TestBackwardsCompatibility (1 test): - Verify ANTHROPIC_API_KEY still works with new system All 191 tests now pass (180 original + 11 new)
GPT-5 and other OpenAI models don't accept the nested format
`{"type": "function", "function": {"name": "..."}}` that Anthropic uses.
Changes:
- Add _get_tool_choice() helper method that returns provider-specific format
- Anthropic: {"type": "function", "function": {"name": "..."}}
- OpenAI/Google/DeepSeek: {"type": "function", "name": "..."}
- Replace all 5 hardcoded tool_choice calls with helper method
- Simplify "auto" tool_choice from {"type": "auto"} to "auto"
Fixes "Unknown parameter: 'tool_choice.function'" error with GPT-5
- Import Optional, Union, ModelResponse types - Add return type annotations to all methods: - _validate_api_key() -> None - _get_tool_choice() -> str - _call_llm() -> Optional[ModelResponse] - Fix _call_llm parameter types: - tools: List[Dict[str, object]] (more specific than Dict) - tool_choice: Union[str, Dict[str, object]] (can be "auto"/"required" or dict) - Fix mutable default arguments in generate_flashcards and generate_from_query: - previous_fronts: Optional[List[str]] = None - deck_examples: Optional[List[Dict[str, str]]] = None No use of Any type - all types are properly specified
LiteLLM's completion() returns Union[ModelResponse, CustomStreamWrapper] but we never use streaming (stream parameter defaults to False). Use cast(ModelResponse, ...) to tell type checker the actual runtime type. Fixes type error: 'ModelResponse | CustomStreamWrapper' is not assignable to 'ModelResponse | None'
Eliminates ~120 lines of repetition across 3 methods by consolidating flashcard extraction logic into _extract_flashcards_from_response(). Changes: - Add _extract_flashcards_from_response() helper method - Refactor generate_flashcards() to use helper (37 lines -> 1 line) - Refactor generate_from_query() to use helper (47 lines -> 14 lines) - Refactor generate_from_note_query() to use helper (39 lines -> 1 line) - Move json import to top of file (was inline in 5 places) - Remove 4 inline "import json" statements Benefits: - Single source of truth for flashcard parsing - Easier to add new features (e.g., validation, logging) - Less error-prone - bug fixes apply everywhere - More maintainable and testable All 191 tests pass
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.