Skip to content

Commit e8f79b9

Browse files
authored
Merge pull request #6 from samefarrar/ankiconnect_check
Ankiconnect check
2 parents e63e90a + 89c03d4 commit e8f79b9

File tree

11 files changed

+1287
-379
lines changed

11 files changed

+1287
-379
lines changed

.asana/01-todo.md

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
# To Do List
2+
3+
## Refactor mcp-ankiconnect based on APSD Review
4+
5+
- [x] **`ankiconnect_client.py` Refactor:**
6+
- [x] Simplify `__init__` timeout handling (expect `TimeoutConfig`).
7+
- [x] Refactor `invoke` into `_send_request_with_retries` and `_parse_response`.
8+
- [x] Remove redundant `else` block in retry loop.
9+
- [x] Remove fallback import logic for `config`.
10+
- [x] **`server.py` Refactor:**
11+
- [x] Extract field processing from `add_note` into `_process_field_content`.
12+
- [x] Extract formatting from `fetch_due_cards_for_review` into `_format_cards_for_llm`.
13+
- [x] Extract query building/formatting from `get_examples` into helpers (`_build_example_query`, `_format_example_notes`).
14+
- [ ] **`tests/` Updates:**
15+
- [ ] Add specific tests for new helper functions in `server.py`.
16+
- [ ] Consider using fixtures or `async with` for client cleanup in tests.

.asana/02-progress.md

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
# Progress Log
2+
3+
## 2025-04-28
4+
5+
- Started refactoring `mcp-ankiconnect` based on APSD review.
6+
- Completed `ankiconnect_client.py` refactor (timeout handling, invoke method, imports).
7+
- Completed `server.py` refactor (extracting helpers for `add_note`, `fetch_due_cards_for_review`, `get_examples`).
8+
- **In Progress:** `tests/` updates (adding tests for new server helper functions).

.gitignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ wheels/
1010
.venv
1111
.aider*
1212
.env
13+
.asana/
1314

1415
# DS Store
1516
.DS_Store

CLAUDE.md

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
You have a .asana folder, with numbered entries for your todo list, progress and implementation documentation.
2+
3+
YOU MUST check this, and keep it updated as you work.
4+
5+
```
6+
01-todo.md # Detailed todo list
7+
02-progress.md # Progress report
8+
```
9+
10+
## Project Management
11+
- Use `uv` for Python package management and running tools (NEVER pip)
12+
- Add packages: `uv add package_name`
13+
- Remove packages: `uv remove package_name`
14+
- Running Python tools: `uv run <tool>`
15+
- Run single test: `uv run pytest tests/path_to_test.py::TestClass::test_method -v`
16+
- Type checking: `uv run pyright .`
17+
- Linting: `uv run ruff check .`
18+
- Run formatter: `uv run ruff format .`
19+
- Async testing: use anyio, not asyncio
20+
21+
## Code Style
22+
- Use type hints for function signatures
23+
- Functions must be small and focused
24+
- Error handling: Use try/except with specific exceptions
25+
- Document functions with docstrings (include "why" not just "what")
26+
- Use f-strings for string formatting
27+
- Max line length: 88 characters
28+
- Program in the spirit of A Philosophy of Software Design by John Ousterhout.
29+
- Explicit None checks for Optional types
30+
- YOU MUST add regression tests when your encounter a bug. This should be the first thing you do when you are told about a bug.
31+
32+
### Coding Guidelines
33+
34+
* **Testing:**
35+
* Write code that is easy to test.
36+
* Tests are an integral part of the development process.
37+
* Start with failing tests and then write code to make them pass (Test-Driven Development principles).
38+
* Unit tests should be next to the functionality they test. So in the src/blog/main.py file.
39+
* Integration tests should be placed in the tests directory.
40+
* AVOID mocks, unless they are requested.
41+
* **Performance:**
42+
* Optimize for readability first.
43+
* Performance optimization should only be considered when explicitly required or requested.
44+
* **Iterative Development:**
45+
* Start with the minimum viable solution.
46+
* Continuously question if a feature or solution is truly necessary.
47+
* Build incrementally based on actual needs, not hypothetical future requirements.
48+
* **Type Annotations:** Type annotations are **required** in our codebase. This helps with code readability, maintainability, and early error detection.

mcp_ankiconnect/ankiconnect_client.py

Lines changed: 135 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -9,18 +9,37 @@
99
ANKI_CONNECT_URL,
1010
ANKI_CONNECT_VERSION,
1111
TIMEOUTS,
12+
TimeoutConfig, # Import TimeoutConfig
1213
)
1314
except ImportError:
14-
from config import (
15-
ANKI_CONNECT_URL,
16-
ANKI_CONNECT_VERSION,
17-
TIMEOUTS,
18-
)
15+
# Attempt to import TimeoutConfig from config if the primary import fails
16+
try:
17+
from config import (
18+
ANKI_CONNECT_URL,
19+
ANKI_CONNECT_VERSION,
20+
TIMEOUTS,
21+
TimeoutConfig, # Import TimeoutConfig
22+
)
23+
except ImportError:
24+
# Handle cases where config.py might not define TimeoutConfig directly
25+
# or provide a fallback if necessary. This might indicate a setup issue.
26+
# For now, we assume TimeoutConfig is available via one of the paths.
27+
# If TimeoutConfig is defined *within* config.py, this structure is fine.
28+
# If it's imported *into* config.py, ensure the original source is in sys.path.
29+
# Re-raising might be appropriate if TimeoutConfig is essential and missing.
30+
raise ImportError("Could not import AnkiConnect configuration including TimeoutConfig.")
31+
1932

2033
from pydantic import BaseModel, Field
2134

2235
logger = logging.getLogger(__name__)
2336

37+
# --- Custom Exception ---
38+
class AnkiConnectionError(Exception):
39+
"""Raised when the client cannot connect to the AnkiConnect server after retries."""
40+
pass
41+
# --- End Custom Exception ---
42+
2443
class AnkiAction(str, Enum):
2544
DECK_NAMES = "deckNames"
2645
FIND_CARDS = "findCards"
@@ -41,110 +60,161 @@ class AnkiConnectRequest(BaseModel):
4160
version: int = 6
4261
params: dict = Field(default_factory=dict)
4362

63+
# Use model_dump for Pydantic v2+
64+
def to_dict(self):
65+
return self.model_dump(exclude_unset=True)
66+
67+
4468
class AnkiConnectClient:
4569
def __init__(self, base_url: str = ANKI_CONNECT_URL):
4670
self.base_url = base_url
47-
self.client = httpx.AsyncClient(timeout=TIMEOUTS)
48-
logger.info(f"Initialized AnkiConnect client with base URL: {base_url}")
49-
50-
async def invoke(self, action: str, **params) -> Any:
71+
# Ensure TIMEOUTS is correctly passed or use httpx.Timeout object
72+
# Assuming TIMEOUTS is a NamedTuple like TimeoutConfig(connect=5.0, read=120.0, write=30.0)
73+
# Check if TIMEOUTS is an instance of the specific TimeoutConfig NamedTuple
74+
if isinstance(TIMEOUTS, TimeoutConfig): # Check against the specific class
75+
# Convert NamedTuple to httpx.Timeout object
76+
timeout_config = httpx.Timeout(TIMEOUTS.connect, read=TIMEOUTS.read, write=TIMEOUTS.write)
77+
else:
78+
# If TIMEOUTS is not the expected NamedTuple, log a warning and use it directly.
79+
# This assumes it might be a float, dict, or httpx.Timeout object already.
80+
# Consider adding more specific type checks if needed.
81+
logger.warning(f"TIMEOUTS config is not a TimeoutConfig NamedTuple (type: {type(TIMEOUTS)}). Using value directly.")
82+
# Assume it's already in a format httpx understands (like float or httpx.Timeout)
83+
timeout_config = TIMEOUTS # Or provide a default httpx.Timeout if TIMEOUTS might be invalid
84+
85+
self.client = httpx.AsyncClient(base_url=base_url, timeout=timeout_config) # Set base_url here
86+
logger.info(f"Initialized AnkiConnect client with base URL: {self.base_url}")
87+
88+
async def invoke(self, action: AnkiAction, **params) -> Any: # Use AnkiAction enum
5189
request = AnkiConnectRequest(
5290
action=action,
53-
version=ANKI_CONNECT_VERSION,
91+
# version=ANKI_CONNECT_VERSION, # version is now in AnkiConnectRequest default
5492
params=params
5593
)
5694

57-
logger.debug(f"Invoking AnkiConnect action: {action} with params: {params}")
95+
logger.debug(f"Invoking AnkiConnect action: {action.value} with params: {params}")
5896

5997
retries = 3
98+
last_exception = None # Keep track of the last exception for the final error message
99+
60100
for attempt in range(retries):
61101
try:
62102
response = await self.client.post(
63-
self.base_url,
64-
json=request.model_dump()
103+
"/", # POST to base_url root
104+
json=request.to_dict() # Use the method to get dict
65105
)
106+
response.raise_for_status() # Check for HTTP 4xx/5xx errors first
107+
108+
# Successful request, break retry loop
66109
break
67-
except httpx.TimeoutException as e:
110+
# --- Catch specific connection errors ---
111+
except (httpx.ConnectError, httpx.TimeoutException, httpx.NetworkError) as e: # Added NetworkError
112+
last_exception = e
113+
logger.warning(f"Attempt {attempt + 1}/{retries} failed for action {action.value}: {e}")
68114
if attempt == retries - 1:
69-
raise RuntimeError(f"Unable to connect to Anki after {retries} attempts. Please ensure Anki is running and the AnkiConnect plugin is installed.")
70-
await asyncio.sleep(2 ** attempt) # Exponential backoff: 1, 2, 4 seconds
71-
continue
115+
# Raise custom error after all retries failed
116+
error_message = (
117+
f"Unable to connect to AnkiConnect at {self.base_url} after {retries} attempts. "
118+
f"Please ensure Anki is running and the AnkiConnect add-on is installed and enabled. "
119+
f"Last error: {last_exception}"
120+
)
121+
logger.error(error_message)
122+
raise AnkiConnectionError(error_message) from last_exception
123+
# Exponential backoff: 1, 2, 4 seconds
124+
backoff_time = 2 ** attempt
125+
logger.info(f"Retrying in {backoff_time} seconds...")
126+
await asyncio.sleep(backoff_time)
127+
continue # Go to next retry attempt
128+
# --- End connection error handling ---
129+
except httpx.HTTPStatusError as e:
130+
# Handle non-connection HTTP errors (like 403 Forbidden, 500 Internal Server Error from AnkiConnect)
131+
logger.error(f"HTTP error invoking {action.value}: Status {e.response.status_code}, Response: {e.response.text}")
132+
# Reraise as a runtime error, potentially including response body
133+
raise RuntimeError(f"AnkiConnect request failed with status {e.response.status_code}: {e.response.text}") from e
134+
except Exception as e:
135+
# Catch any other unexpected errors during the request/response cycle
136+
logger.exception(f"Unexpected error during AnkiConnect invoke action '{action.value}': {e}")
137+
# Reraise as a generic runtime error or a more specific custom error if identifiable
138+
raise RuntimeError(f"An unexpected error occurred during the AnkiConnect request: {e}") from e
139+
else:
140+
# This else block executes if the loop completed without break (i.e., all retries failed)
141+
# This should theoretically be covered by the retry == retries - 1 check inside the loop,
142+
# but adding it for robustness in case of unexpected loop exit.
143+
if last_exception:
144+
error_message = f"AnkiConnect action {action.value} failed after {retries} retries. Last error: {last_exception}"
145+
logger.error(error_message)
146+
raise AnkiConnectionError(error_message) from last_exception
147+
else:
148+
# Should not happen if loop finishes, but handle defensively
149+
error_message = f"AnkiConnect action {action.value} failed after {retries} retries for an unknown reason."
150+
logger.error(error_message)
151+
raise RuntimeError(error_message)
152+
153+
154+
# --- Process successful response ---
72155
try:
73-
response.raise_for_status()
156+
# Decode the JSON response (synchronous in httpx)
157+
response_data = response.json()
158+
159+
# Check if the response is the expected dictionary format or just the result
160+
if isinstance(response_data, dict) and 'result' in response_data and 'error' in response_data:
161+
# Standard format, validate directly
162+
anki_response = AnkiConnectResponse.model_validate(response_data)
163+
else:
164+
# Assume response_data is the result itself (e.g., a list for deckNames)
165+
logger.debug(f"Received direct result payload for action {action.value}. Wrapping in AnkiConnectResponse.")
166+
anki_response = AnkiConnectResponse(result=response_data, error=None)
74167

75-
anki_response = AnkiConnectResponse.model_validate(response.json())
76168
if anki_response.error:
77-
logger.error(f"AnkiConnect error for action {action}: {anki_response.error}")
169+
logger.error(f"AnkiConnect API returned error for action {action.value}: {anki_response.error}")
170+
# This is an error reported by the AnkiConnect API itself
78171
raise ValueError(f"AnkiConnect error: {anki_response.error}")
79172

173+
logger.debug(f"AnkiConnect action {action.value} successful.")
80174
return anki_response.result
81175

82-
except httpx.HTTPError as e:
83-
logger.error(f"HTTP error while invoking {action}: {str(e)}")
84-
raise RuntimeError(f"Failed to communicate with AnkiConnect: {str(e)}") from e
85176
except ValueError as e:
86-
# Re-raise ValueError (from AnkiConnect errors) directly
177+
# Re-raise ValueError (from AnkiConnect errors or JSON parsing issues) directly
178+
logger.error(f"Error processing AnkiConnect response for action {action.value}: {e}")
87179
raise
88180
except Exception as e:
89-
logger.error(f"Unexpected error while invoking {action}: {str(e)}")
90-
raise RuntimeError(f"Unexpected error during AnkiConnect operation: {str(e)}") from e
181+
logger.exception(f"Unexpected error processing AnkiConnect response for {action.value}: {str(e)}")
182+
raise RuntimeError(f"Unexpected error processing AnkiConnect response: {str(e)}") from e
183+
# --- End response processing ---
91184

185+
186+
# --- Wrapper methods ---
187+
# Remove redundant try/except blocks, rely on invoke's error handling
92188
async def cards_info(self, card_ids: List[int]) -> List[dict]:
93-
try:
94-
return await self.invoke(AnkiAction.CARDS_INFO, cards=card_ids)
95-
except Exception as e:
96-
raise RuntimeError(f"Error getting cards info: {str(e)}") from e
189+
return await self.invoke(AnkiAction.CARDS_INFO, cards=card_ids)
97190

98191
async def deck_names(self) -> List[str]:
99-
try:
100-
return await self.invoke(AnkiAction.DECK_NAMES)
101-
except Exception as e:
102-
if isinstance(e, RuntimeError) and "Unable to connect to Anki" in str(e):
103-
raise
104-
raise RuntimeError(f"Error getting deck names: {str(e)}") from e
192+
return await self.invoke(AnkiAction.DECK_NAMES)
105193

106194
async def find_cards(self, query: str) -> List[int]:
107-
try:
108-
return await self.invoke(AnkiAction.FIND_CARDS, query=query)
109-
except Exception as e:
110-
raise RuntimeError(f"Error finding cards: {str(e)}") from e
195+
return await self.invoke(AnkiAction.FIND_CARDS, query=query)
111196

112197
async def answer_cards(self, answers: List[dict]) -> List[bool]:
113-
try:
114-
return await self.invoke(AnkiAction.ANSWER_CARDS, answers=answers)
115-
except Exception as e:
116-
raise RuntimeError(f"Error answering cards: {str(e)}") from e
198+
# AnkiConnect expects list of {"cardId": int, "ease": int}
199+
# Ensure the input format matches or convert here if needed
200+
return await self.invoke(AnkiAction.ANSWER_CARDS, answers=answers)
117201

118202
async def model_field_names(self, model_name: str) -> List[str]:
119-
try:
120-
return await self.invoke(AnkiAction.MODEL_FIELD_NAMES, modelName=model_name)
121-
except Exception as e:
122-
raise RuntimeError(f"Error getting model field names: {str(e)}") from e
203+
return await self.invoke(AnkiAction.MODEL_FIELD_NAMES, modelName=model_name)
123204

124205
async def model_names(self) -> List[str]:
125-
try:
126-
return await self.invoke(AnkiAction.MODEL_NAMES)
127-
except Exception as e:
128-
raise RuntimeError(f"Error getting model names: {str(e)}") from e
206+
return await self.invoke(AnkiAction.MODEL_NAMES)
129207

130208
async def find_notes(self, query: str) -> List[int]:
131-
try:
132-
return await self.invoke(AnkiAction.FIND_NOTES, query=query)
133-
except Exception as e:
134-
raise RuntimeError(f"Error finding notes: {str(e)}") from e
209+
return await self.invoke(AnkiAction.FIND_NOTES, query=query)
135210

136211
async def add_note(self, note: dict) -> int:
137-
try:
138-
return await self.invoke(AnkiAction.ADD_NOTE, note=note)
139-
except Exception as e:
140-
raise RuntimeError(f"Error adding note: {str(e)}") from e
141-
212+
# Note structure should match AnkiConnect requirements:
213+
# {"deckName": ..., "modelName": ..., "fields": {...}, "tags": [...], "options": {...}}
214+
return await self.invoke(AnkiAction.ADD_NOTE, note=note)
142215

143216
async def notes_info(self, note_ids: List[int]) -> List[dict]:
144-
try:
145-
return await self.invoke(AnkiAction.NOTES_INFO, notes=note_ids)
146-
except Exception as e:
147-
raise RuntimeError(f"Error getting notes info: {str(e)}") from e
217+
return await self.invoke(AnkiAction.NOTES_INFO, notes=note_ids)
148218

149219
async def close(self):
150220
await self.client.aclose()

0 commit comments

Comments
 (0)