diff options
| -rw-r--r-- | CHANGELOG.md | 9 | ||||
| -rw-r--r-- | src/ollama_web/app.py | 34 | ||||
| -rw-r--r-- | src/ollama_web/llm.py | 181 | ||||
| -rw-r--r-- | src/ollama_web/tools/search.py | 13 | ||||
| -rw-r--r-- | tests/test_llm.py | 101 | ||||
| -rw-r--r-- | tests/test_logging.py | 88 | ||||
| -rw-r--r-- | tests/test_settings.py | 25 | ||||
| -rw-r--r-- | tests/test_tools.py | 49 |
8 files changed, 248 insertions, 252 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index 1381129..f7d0b69 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,15 @@ ## [Unreleased] +### Changed + +- 使用されていない非ストリーミングのチャット処理を削除し、ツール呼び出し関連のテストを実運用のストリーミング処理へ統合しました。 + +### Fixed + +- `server.log`を`OLLAMA_WEB_DATA_DIR`の親ではなくデータディレクトリ直下へ保存し、アプリケーションの起動・終了に合わせてログハンドラーを開閉するようにしました。 +- DuckDuckGo検索で発生した例外を空の検索結果として握りつぶさず、検索語を記録しない診断ログを残してツール実行層へ伝播するようにしました。 + ## [0.5.0] - 2026-08-16 ### Added diff --git a/src/ollama_web/app.py b/src/ollama_web/app.py index eeb3bae..9792c7b 100644 --- a/src/ollama_web/app.py +++ b/src/ollama_web/app.py @@ -3,6 +3,8 @@ from __future__ import annotations import logging +from collections.abc import AsyncIterator +from contextlib import asynccontextmanager from pathlib import Path from typing import Any @@ -42,17 +44,29 @@ class NoCacheStaticFiles(StaticFiles): _STATIC_DIR = Path(__file__).resolve().parent / "static" +@asynccontextmanager +async def _lifespan(app: Starlette) -> AsyncIterator[None]: + """Keep the application log handler scoped to the ASGI lifespan.""" + log_dir = Path(app.state.settings.data_dir).resolve() + log_dir.mkdir(parents=True, exist_ok=True) + handler = logging.FileHandler(str(log_dir / "server.log"), encoding="utf-8") + handler.setFormatter( + logging.Formatter("%(asctime)s - %(name)s - %(levelname)s - %(message)s") + ) + logger = logging.getLogger("ollama_web") + previous_level = logger.level + logger.setLevel(logging.INFO) + logger.addHandler(handler) + try: + yield + finally: + logger.removeHandler(handler) + handler.close() + logger.setLevel(previous_level) + + def create_app() -> Starlette: """Build and return the configured Starlette application.""" - # Ensure application logs are written alongside uvicorn access logs. - log_path = Path(settings.data_dir).resolve().parent / "server.log" - log_path.parent.mkdir(parents=True, exist_ok=True) - handler = logging.FileHandler(str(log_path), encoding="utf-8") - handler.setFormatter(logging.Formatter("%(asctime)s - %(name)s - %(levelname)s - %(message)s")) - root = logging.getLogger() - root.setLevel(logging.INFO) - root.addHandler(handler) - routes = [ Route("/login", pages_route.login, name="login_page"), Route("/api/auth/login", auth_route.login, methods=["POST"], name="auth_login"), @@ -139,7 +153,7 @@ def create_app() -> Starlette: ) ) - app = Starlette(routes=routes, middleware=middleware) + app = Starlette(routes=routes, middleware=middleware, lifespan=_lifespan) app.state.settings = settings app.state.session_store = SessionStore(settings.data_dir) app.state.tool_names = ["web_search", "scrape_url", "search_and_fetch", "pdf_to_text"] diff --git a/src/ollama_web/llm.py b/src/ollama_web/llm.py index 4951b36..187b5e7 100644 --- a/src/ollama_web/llm.py +++ b/src/ollama_web/llm.py @@ -132,14 +132,6 @@ def _embed_images_in_content(message: Message) -> Message: return Message(**kwargs) -def _tool_calls(response: Any) -> list[dict[str, Any]]: - """Extract tool calls from an ollama response in a tolerant manner.""" - msg = getattr(response, "message", None) or ( - response.get("message") if isinstance(response, dict) else None - ) - return _tool_calls_from_message(msg) - - def _tool_calls_from_message(msg: Any) -> list[dict[str, Any]]: """Extract tool calls from a message object/dict.""" if msg is None: @@ -166,18 +158,6 @@ def _tool_calls_from_message(msg: Any) -> list[dict[str, Any]]: return out -def _assistant_message(response: Any) -> Message: - """Convert a non-streaming response into a Message for re-submission.""" - msg = getattr(response, "message", None) or ( - response.get("message") if isinstance(response, dict) else None - ) - if isinstance(msg, dict): - return Message(**msg) - if msg is not None: - return cast(Message, msg) - return Message(role="assistant", content="") - - def _token_counts(response: Any) -> dict[str, int] | None: """Extract prompt_eval_count and eval_count from an ollama response.""" if response is None: @@ -193,161 +173,6 @@ def _token_counts(response: Any) -> dict[str, int] | None: return {"prompt_eval_count": int(prompt_eval), "eval_count": int(eval_count)} -def chat_with_tools( - messages: list[Message], - model: str, - *, - think: bool | str | None = None, - registry: ToolRegistry | None = None, - host: str | None = None, - capabilities: set[str] | None = None, - options: dict[str, Any] | None = None, - language: str | None = None, -) -> Iterator[dict[str, Any]]: - """Run a multi-round tool-calling chat and yield progress events. - - Events are dicts with a ``type`` key: - - - ``{"type": "tool_start", "name": str, "arguments": dict}`` - - ``{"type": "tool_end", "name": str, "result": str}`` - - ``{"type": "delta", "content": str}`` - - ``{"type": "thinking", "content": str}`` - - ``{"type": "done", "prompt_eval_count": int, "eval_count": int}`` - - ``{"type": "error", "message": str}`` - - The final assistant message is streamed token-by-token via ``delta`` events. - """ - import logging - - logger = logging.getLogger("ollama_web.llm") - - caps = capabilities or set() - raw_client = get_client(host) - reg = registry or default_registry() - tools: list[ToolCallable] = [cast(Any, t) for t in reg.ollama_tools] - effective_tools = tools if "tools" in caps else [] - effective_think = think if "thinking" in caps else None - - # Non-vision models cannot use the ``images`` field; embed them as base64. - if "vision" not in caps: - messages = [_embed_images_in_content(m) if _is_image_message(m) else m for m in messages] - - cumulative_tool_chars = 0 - - rounds = 0 - while rounds < MAX_TOOL_ROUNDS: - rounds += 1 - logger.info( - "tool round %d/%d start messages=%d model=%s cumulative_tool_chars=%d", - rounds, - MAX_TOOL_ROUNDS, - len(messages), - model, - cumulative_tool_chars, - ) - try: - resp = cast(Any, raw_client).chat( - model=model, - messages=messages, - tools=effective_tools, - think=effective_think, - options=options or None, - stream=False, - ) - except Exception as exc: # noqa: BLE001 - logger.warning("ollama chat call failed: %s", exc) - yield {"type": "error", "message": str(exc)} - return - - calls = _tool_calls(resp) - tokens = _token_counts(resp) - if not calls: - # No tool calls: stream the final answer for UI effect. - content = _assistant_content(resp) - thinking = _assistant_thinking(resp) - if thinking: - yield {"type": "thinking", "content": thinking} - if content: - yield {"type": "delta", "content": content} - done_event: dict[str, Any] = {"type": "done"} - if tokens: - done_event.update(tokens) - yield done_event - return - - # Append the assistant message that requested tool calls. - messages.append(_assistant_message(resp)) - - # Execute each requested tool and append tool-role messages. - tool_results: list[tuple[str, str]] = [] - for call in calls: - name = call.get("name", "") - args = call.get("arguments", {}) - - yield {"type": "tool_start", "name": name, "arguments": _safe_tool_arguments(args)} - result = reg.execute(name, args if isinstance(args, dict) else json.dumps(args)) - result = _safe_tool_result(_trim_tool_result(result, settings.max_tool_result_chars)) - yield {"type": "tool_end", "name": name, "result": result} - tool_results.append((name, result)) - - if sum(len(r) for _, r in tool_results) > MAX_TOOL_CONTEXT_CHARS: - budget = MAX_TOOL_CONTEXT_CHARS - len(tool_results) * 200 - max_per_tool = budget // max(1, len(tool_results)) - logger.warning( - "tool results too large (%d chars), truncating to %d chars per tool", - sum(len(r) for _, r in tool_results), - max_per_tool, - ) - tool_results = [(n, _trim_tool_result(r, max_per_tool)) for n, r in tool_results] - - cumulative_tool_chars += sum(len(r) for _, r in tool_results) - for name, result in tool_results: - messages.append(Message(role="tool", tool_name=name, content=result)) - - # Exceeded tool rounds: ask for a final answer without tools. - logger.info( - ( - "max tool rounds exceeded, requesting final answer without tools " - "(cumulative_tool_chars=%d)" - ), - cumulative_tool_chars, - ) - yield { - "type": "status", - "message": t("status.tool_limit_reached", lang=language), - } - messages.append( - Message( - role="user", - content=get_prompt("tool_limit_instruction", lang=language), - ) - ) - try: - resp = cast(Any, raw_client).chat( - model=model, - messages=messages, - tools=[], - think=effective_think, - options=options or None, - stream=False, - ) - except Exception as exc: # noqa: BLE001 - logger.warning("final ollama chat call failed: %s", exc) - yield {"type": "error", "message": str(exc)} - return - tokens = _token_counts(resp) - content = _assistant_content(resp) - thinking = _assistant_thinking(resp) - if thinking: - yield {"type": "thinking", "content": thinking} - if content: - yield {"type": "delta", "content": content} - done_event_final: dict[str, Any] = {"type": "done"} - if tokens: - done_event_final.update(tokens) - yield done_event_final - - def _trim_tool_result(result: str, limit: int) -> str: """Truncate ``result`` to ``limit`` chars while keeping it readable. @@ -684,12 +509,6 @@ def _assistant_thinking(response: Any) -> str: return str(getattr(msg, "thinking", "") or "") -def _chunk_text(text: str, size: int) -> Iterator[str]: - """Yield successive chunks of ``text`` up to ``size`` characters.""" - for i in range(0, len(text), size): - yield text[i : i + size] - - def _assistant_message_chunk(full_message: dict[str, Any]) -> Message | None: """Build an assistant ``Message`` from the accumulated streamed message. diff --git a/src/ollama_web/tools/search.py b/src/ollama_web/tools/search.py index 761f182..773f7f3 100644 --- a/src/ollama_web/tools/search.py +++ b/src/ollama_web/tools/search.py @@ -2,10 +2,14 @@ from __future__ import annotations +import logging + from ddgs import DDGS from ..config import settings +logger = logging.getLogger(__name__) + def web_search(query: str, max_results: int | None = None) -> list[dict[str, str]]: """Search the web using DuckDuckGo and return compact result summaries. @@ -16,7 +20,8 @@ def web_search(query: str, max_results: int | None = None) -> list[dict[str, str Returns: A list of dictionaries with keys 'title', 'href' and 'body' for each - result. Returns an empty list if the search fails or no results are found. + result. Returns an empty list if no results are found. Provider failures + are logged and propagated to the tool execution layer. """ if not query or not query.strip(): return [] @@ -37,7 +42,9 @@ def web_search(query: str, max_results: int | None = None) -> list[dict[str, str } ) except Exception: # noqa: BLE001 - # Swallow network/runtime errors so the chat loop can continue. - return [] + # Preserve the provider exception for the tool layer while recording + # the traceback without logging the potentially sensitive query. + logger.exception("DuckDuckGo web search failed") + raise return results[:limit] diff --git a/tests/test_llm.py b/tests/test_llm.py index edd9d61..0e1b146 100644 --- a/tests/test_llm.py +++ b/tests/test_llm.py @@ -9,13 +9,11 @@ from typing import Any import pytest from ollama import Message from ollama._types import Image as OllamaImage -from ollama._types import ResponseError from ollama_web.llm import ( _embed_images_in_content, _is_image_message, _token_counts, - chat_with_tools, stream_chat_with_tools, ) from ollama_web.tools.registry import ToolRegistry @@ -44,46 +42,44 @@ def test_embed_images_in_content_adds_base64_markdown() -> None: assert not getattr(embedded, "images", None) -def test_chat_with_tools_skips_tools_for_non_tool_model() -> None: - """Tools are not passed when the model does not advertise tool support.""" - messages = [Message(role="user", content="hi")] - # Use an empty registry so any tool call would fail if requested. +def test_stream_chat_with_tools_disables_unsupported_capabilities( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Unsupported tools and thinking options are not sent to Ollama.""" + from ollama_web import llm + registry = ToolRegistry() - # Without tool capability, the iterator should finish immediately with no - # actual network request. We only verify it accepts the call and does not - # raise due to tool-related setup. - gen = chat_with_tools( - messages, - model="nonexistent-model", - capabilities=set(), - registry=registry, - ) - events = list(gen) - # We expect either an error (model not found) or a done event. - assert any(e.get("type") in {"error", "done"} for e in events) + def lookup(query: str) -> str: + return query + registry.register(lookup) -def test_chat_with_tools_forces_think_none_for_non_thinking_model() -> None: - messages = [Message(role="user", content="hi")] - gen = chat_with_tools( - messages, - model="nonexistent-model", - capabilities=set(), - ) - events = list(gen) - assert any(e.get("type") in {"error", "done"} for e in events) + class FakeClient: + def __init__(self) -> None: + self.calls: list[dict[str, Any]] = [] + def chat(self, **kwargs: Any) -> list[SimpleNamespace]: + self.calls.append(kwargs) + return [SimpleNamespace(message={"content": "done"})] + + client = FakeClient() + monkeypatch.setattr(llm, "get_client", lambda _host=None: client) -def test_stream_chat_with_tools_accepts_capability_set() -> None: - messages = [Message(role="user", content="hi")] - gen = stream_chat_with_tools( - messages, - model="nonexistent-model", - capabilities={"tools", "thinking"}, + events = list( + stream_chat_with_tools( + [Message(role="user", content="hi")], + model="test-model", + think=True, + capabilities=set(), + registry=registry, + ) ) - with pytest.raises(ResponseError): - list(gen) + + assert client.calls[0]["tools"] == [] + assert client.calls[0]["think"] is None + assert client.calls[0]["stream"] is True + assert events[-1] == {"type": "done"} def test_token_counts_extracts_from_dict() -> None: @@ -109,7 +105,7 @@ def test_token_counts_returns_none_when_missing() -> None: assert _token_counts(None) is None -def test_chat_with_tools_disables_tools_for_final_answer_after_limit( +def test_stream_chat_with_tools_disables_tools_for_final_answer_after_limit( monkeypatch: pytest.MonkeyPatch, ) -> None: from ollama_web import llm @@ -128,26 +124,33 @@ def test_chat_with_tools_disables_tools_for_final_answer_after_limit( def __init__(self) -> None: self.calls: list[dict[str, Any]] = [] - def chat(self, **kwargs: Any) -> dict[str, Any]: + def chat(self, **kwargs: Any) -> list[SimpleNamespace]: self.calls.append(kwargs) if len(self.calls) == 1: - return { - "message": { - "role": "assistant", - "content": "", - "tool_calls": [ - {"function": {"name": "lookup", "arguments": {"query": "test"}}} - ], - } - } - return {"message": {"role": "assistant", "content": "final answer"}} + return [ + SimpleNamespace( + message={ + "role": "assistant", + "content": "", + "tool_calls": [ + { + "function": { + "name": "lookup", + "arguments": {"query": "test"}, + } + } + ], + } + ) + ] + return [SimpleNamespace(message={"content": "final answer"})] client = FakeClient() monkeypatch.setattr(llm, "get_client", lambda _host=None: client) monkeypatch.setattr(llm, "MAX_TOOL_ROUNDS", 1) events = list( - llm.chat_with_tools( + llm.stream_chat_with_tools( [Message(role="user", content="research this")], model="test-model", registry=registry, @@ -160,6 +163,8 @@ def test_chat_with_tools_disables_tools_for_final_answer_after_limit( assert len(client.calls) == 2 assert client.calls[0]["tools"] assert client.calls[1]["tools"] == [] + assert client.calls[0]["stream"] is True + assert client.calls[1]["stream"] is True assert any(event.get("type") == "status" for event in events) assert any(event == {"type": "delta", "content": "final answer"} for event in events) assert events[-1] == {"type": "done"} diff --git a/tests/test_logging.py b/tests/test_logging.py new file mode 100644 index 0000000..ef6fbd2 --- /dev/null +++ b/tests/test_logging.py @@ -0,0 +1,88 @@ +"""Tests for application file logging.""" + +from __future__ import annotations + +import json +import logging +from pathlib import Path +from unittest.mock import patch + +import pytest +from starlette.testclient import TestClient + +from ollama_web.app import create_app +from ollama_web.config import settings +from ollama_web.tools.registry import default_registry + + +def test_server_log_is_scoped_to_data_dir_and_app_lifespan( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + data_dir = tmp_path / "data" + monkeypatch.setattr(settings, "data_dir", str(data_dir)) + app_logger = logging.getLogger("ollama_web") + original_handlers = tuple(app_logger.handlers) + original_level = app_logger.level + + app = create_app() + log_path = data_dir / "server.log" + assert not log_path.exists() + + with TestClient(app): + assert len(app_logger.handlers) == len(original_handlers) + 1 + logging.getLogger("ollama_web.test").info("lifespan log marker") + + assert tuple(app_logger.handlers) == original_handlers + assert app_logger.level == original_level + assert log_path.read_text(encoding="utf-8").count("lifespan log marker") == 1 + + # Closing the lifespan must release the file on Windows as well. + log_path.unlink() + assert not log_path.exists() + + +def test_repeated_app_lifespans_do_not_duplicate_log_handlers( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + data_dir = tmp_path / "data" + monkeypatch.setattr(settings, "data_dir", str(data_dir)) + + with TestClient(create_app()): + logging.getLogger("ollama_web.test").info("first app marker") + with TestClient(create_app()): + logging.getLogger("ollama_web.test").info("second app marker") + + content = (data_dir / "server.log").read_text(encoding="utf-8") + assert content.count("first app marker") == 1 + assert content.count("second app marker") == 1 + + +def test_web_search_provider_error_is_written_to_server_log( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + data_dir = tmp_path / "data" + monkeypatch.setattr(settings, "data_dir", str(data_dir)) + + class FailingDDGS: + def __enter__(self): + return self + + def __exit__(self, *args): + return None + + def text(self, query, max_results=5): + raise RuntimeError("403 Forbidden") + + with patch("ollama_web.tools.search.DDGS", FailingDDGS), TestClient(create_app()): + result = json.loads( + default_registry().execute("web_search", {"query": "private search terms"}) + ) + + assert result == {"error": "RuntimeError: 403 Forbidden"} + content = (data_dir / "server.log").read_text(encoding="utf-8") + assert "DuckDuckGo web search failed" in content + assert "RuntimeError: 403 Forbidden" in content + assert "private search terms" not in content diff --git a/tests/test_settings.py b/tests/test_settings.py index 0f334f4..d19f64d 100644 --- a/tests/test_settings.py +++ b/tests/test_settings.py @@ -4,6 +4,7 @@ from __future__ import annotations import json from pathlib import Path +from types import SimpleNamespace from typing import Any import pytest @@ -151,24 +152,28 @@ def test_options_are_forwarded_to_tool_and_final_rounds(monkeypatch: pytest.Monk def __init__(self) -> None: self.calls: list[dict[str, Any]] = [] - def chat(self, **kwargs: Any) -> dict[str, Any]: + def chat(self, **kwargs: Any) -> list[SimpleNamespace]: self.calls.append(kwargs) if len(self.calls) == 1: - return { - "message": { - "role": "assistant", - "content": "", - "tool_calls": [{"function": {"name": "missing_tool", "arguments": {}}}], - } - } - return {"message": {"role": "assistant", "content": "done"}} + return [ + SimpleNamespace( + message={ + "role": "assistant", + "content": "", + "tool_calls": [ + {"function": {"name": "missing_tool", "arguments": {}}} + ], + } + ) + ] + return [SimpleNamespace(message={"content": "done"})] client = FakeClient() monkeypatch.setattr(llm, "get_client", lambda _host=None: client) monkeypatch.setattr(llm, "MAX_TOOL_ROUNDS", 1) options = {"temperature": 0.8, "num_ctx": 8192, "top_p": 0.9} events = list( - llm.chat_with_tools( + llm.stream_chat_with_tools( [Message(role="user", content="hello")], "test-model", capabilities={"tools"}, diff --git a/tests/test_tools.py b/tests/test_tools.py index a94ea52..387cbe2 100644 --- a/tests/test_tools.py +++ b/tests/test_tools.py @@ -114,6 +114,55 @@ def test_web_search_mocked(): assert result[0]["title"] == "T" +def test_web_search_returns_empty_list_for_no_provider_results() -> None: + from ollama_web.tools import search as search_mod + + class EmptyDDGS: + def __enter__(self): + return self + + def __exit__(self, *args): + return None + + def text(self, query, max_results=5): + return [] + + with patch.object(search_mod, "DDGS", EmptyDDGS): + assert search_mod.web_search("nothing found") == [] + + +def test_web_search_logs_and_propagates_provider_errors( + caplog: pytest.LogCaptureFixture, +) -> None: + from ollama_web.tools import search as search_mod + + class FailingDDGS: + def __enter__(self): + return self + + def __exit__(self, *args): + return None + + def text(self, query, max_results=5): + raise RuntimeError("403 Forbidden") + + with ( + patch.object(search_mod, "DDGS", FailingDDGS), + caplog.at_level("ERROR", logger="ollama_web.tools.search"), + ): + with pytest.raises(RuntimeError, match="403 Forbidden"): + search_mod.web_search("private search terms") + + assert "DuckDuckGo web search failed" in caplog.text + assert "private search terms" not in caplog.text + + registry = ToolRegistry() + registry.register(search_mod.web_search) + with patch.object(search_mod, "DDGS", FailingDDGS): + result = json.loads(registry.execute("web_search", {"query": "test"})) + assert result == {"error": "RuntimeError: 403 Forbidden"} + + # --------------------------------------------------------------------------- # scrape_url tests (existing + pagination) # --------------------------------------------------------------------------- |