From 74dea65f21e006ef9ec45f642510b2c83ca81b27 Mon Sep 17 00:00:00 2001 From: Yiorgis Gozadinos Date: Mon, 18 May 2026 14:50:52 +0300 Subject: [PATCH] analysis.model inherits qa.model when unset; per-skill vision gate --- evaluations/evaluations/benchmark.py | 9 ++- .../haiku/rag/agents/analysis/agent.py | 2 +- haiku_rag_slim/haiku/rag/config/models.py | 16 ++-- haiku_rag_slim/haiku/rag/skills/_tools.py | 19 +++-- haiku_rag_slim/haiku/rag/skills/analysis.py | 1 + haiku_rag_slim/haiku/rag/skills/rag.py | 4 +- tests/test_config.py | 77 +++++++++++++++++++ tests/test_picture_in_context.py | 77 +++++++++++++++++++ tests/test_skill_tools.py | 1 + 9 files changed, 188 insertions(+), 18 deletions(-) diff --git a/evaluations/evaluations/benchmark.py b/evaluations/evaluations/benchmark.py index 4392b9c1..bbedd446 100644 --- a/evaluations/evaluations/benchmark.py +++ b/evaluations/evaluations/benchmark.py @@ -363,7 +363,14 @@ async def run_qa_benchmark( ] judge_config = judge_model or DEFAULT_JUDGE_MODEL - skill_config = (skill_model or config.qa.model) if target != "qa" else None + if target == "qa": + skill_config = None + elif target == "analysis-skill": + # Mirror the skill-code resolver: explicit analysis.model wins, + # else fall back to qa.model. + skill_config = skill_model or config.analysis.model or config.qa.model + else: + skill_config = skill_model or config.qa.model db = spec.db_path(db_path) citation_evaluator: Evaluator | None = None diff --git a/haiku_rag_slim/haiku/rag/agents/analysis/agent.py b/haiku_rag_slim/haiku/rag/agents/analysis/agent.py index fa242c90..da656299 100644 --- a/haiku_rag_slim/haiku/rag/agents/analysis/agent.py +++ b/haiku_rag_slim/haiku/rag/agents/analysis/agent.py @@ -20,7 +20,7 @@ def create_analysis_agent(config: AppConfig) -> Agent[AnalysisDeps, RawAnalysisR Returns: A pydantic-ai Agent configured for analysis execution. """ - model = get_model(config.analysis.model, config) + model = get_model(config.analysis.model or config.qa.model, config) agent: Agent[AnalysisDeps, RawAnalysisResult] = Agent( # type: ignore[assignment] # ty: ignore[invalid-assignment] model, diff --git a/haiku_rag_slim/haiku/rag/config/models.py b/haiku_rag_slim/haiku/rag/config/models.py index bf7cb4e6..f263e027 100644 --- a/haiku_rag_slim/haiku/rag/config/models.py +++ b/haiku_rag_slim/haiku/rag/config/models.py @@ -116,14 +116,14 @@ class ResearchConfig(BaseModel): class AnalysisConfig(BaseModel): - model: ModelConfig = Field( - default_factory=lambda: ModelConfig( - provider="ollama", - name="gpt-oss", - enable_thinking=False, - temperature=0.0, - ) - ) + """Driving model + sandbox limits for the analysis skill. + + ``model`` defaults to ``None``, meaning "no override — use ``qa.model``." + Consumers resolve via ``config.analysis.model or config.qa.model``. Set + explicitly when the analysis workload wants a different model from QA + (e.g. a stronger model for computational tasks).""" + + model: ModelConfig | None = None code_timeout: float = 60.0 max_output_chars: int = 50_000 diff --git a/haiku_rag_slim/haiku/rag/skills/_tools.py b/haiku_rag_slim/haiku/rag/skills/_tools.py index d887e64d..a204e2fa 100644 --- a/haiku_rag_slim/haiku/rag/skills/_tools.py +++ b/haiku_rag_slim/haiku/rag/skills/_tools.py @@ -7,7 +7,7 @@ from pydantic_ai.messages import ToolReturn from haiku.rag.agents.research.models import Citation from haiku.rag.client import HaikuRAG -from haiku.rag.config.models import AppConfig +from haiku.rag.config.models import AppConfig, ModelConfig from haiku.rag.skills._deps import AnalysisRunDeps, RAGRunDeps from haiku.rag.store.models.chunk import SearchResult from haiku.rag.tools.search import build_binary_parts_from_results @@ -155,12 +155,18 @@ def create_skill_tools( config: AppConfig, state_type: type[BaseModel], tool_names: list[str], + model: ModelConfig, ) -> dict[str, Any]: """Create tool closures for a skill. Returns a dict mapping tool name to async callable. Each tool extracts state from RunContext, calls the shared implementation, - and updates state. + and updates state. ``model`` is the driving model for the skill (e.g. + ``config.qa.model`` for the RAG skill, or + ``config.analysis.model or config.qa.model`` for the analysis skill, + which defaults to ``None`` and inherits QA's model when unconfigured); + its ``vision`` flag gates picture-bytes attachment on the ``search`` + tool. """ tools: dict[str, Any] = {} @@ -173,10 +179,9 @@ def create_skill_tools( """Search the knowledge base using hybrid search (vector + full-text). Returns ranked results with content and metadata. When picture - content is in the result set and the configured QA model is - vision-capable (``qa.model.vision = true``), picture bytes are - attached as ``BinaryContent`` parts so the model sees figures - alongside text. + content is in the result set and the driving skill model is + vision-capable, picture bytes are attached as ``BinaryContent`` + parts so the model sees figures alongside text. Args: query: The search query. @@ -199,7 +204,7 @@ def create_skill_tools( if state: state.searches[query] = results - if not config.qa.model.vision: + if not model.vision: return formatted binary_parts = build_binary_parts_from_results(results) diff --git a/haiku_rag_slim/haiku/rag/skills/analysis.py b/haiku_rag_slim/haiku/rag/skills/analysis.py index b9cedd2f..12aa7804 100644 --- a/haiku_rag_slim/haiku/rag/skills/analysis.py +++ b/haiku_rag_slim/haiku/rag/skills/analysis.py @@ -78,6 +78,7 @@ def create_skill( config, AnalysisState, ["search", "execute_code", "cite"], + model=config.analysis.model or config.qa.model, ) extras = create_skill_extras(db_path, config) diff --git a/haiku_rag_slim/haiku/rag/skills/rag.py b/haiku_rag_slim/haiku/rag/skills/rag.py index e8220859..db2cf8e5 100644 --- a/haiku_rag_slim/haiku/rag/skills/rag.py +++ b/haiku_rag_slim/haiku/rag/skills/rag.py @@ -88,7 +88,9 @@ def create_skill( else: db_path = config.storage.data_dir / "haiku.rag.lancedb" - tools = create_skill_tools(db_path, config, RAGState, _RAG_TOOLS) + tools = create_skill_tools( + db_path, config, RAGState, _RAG_TOOLS, model=config.qa.model + ) extras = create_skill_extras(db_path, config) skill_instructions = instructions() diff --git a/tests/test_config.py b/tests/test_config.py index 0612e948..cdac07df 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -274,3 +274,80 @@ processing: data = load_yaml_config(config_file) cfg = AppConfig.model_validate(data) assert cfg.processing.conversion_options.fetch_remote_images is False + + +def test_analysis_model_defaults_to_none(): + """``AnalysisConfig.model`` is ``None`` by default; consumers resolve via + ``config.analysis.model or config.qa.model``. Keeps the field semantics + simple: ``None`` means "no override, inherit from QA".""" + cfg = AppConfig() + assert cfg.analysis.model is None + + +def test_analysis_model_unset_resolves_to_qa(tmp_path): + """When YAML configures ``qa.model`` and omits ``analysis.model``, the + resolve idiom yields qa.model.""" + cfg = AppConfig.model_validate( + load_yaml_config( + _write( + tmp_path, + """ +qa: + model: + provider: openai + name: my/qwen + base_url: http://example/v1 + vision: true +""", + ) + ) + ) + assert cfg.qa.model.name == "my/qwen" + assert cfg.analysis.model is None + resolved = cfg.analysis.model or cfg.qa.model + assert resolved.name == "my/qwen" + assert resolved.vision is True + + +def test_analysis_other_fields_keep_defaults_with_unset_model(tmp_path): + """``analysis`` may contain non-model overrides (e.g. ``code_timeout``) + without a ``model`` key; model stays None, other fields take user values.""" + cfg = AppConfig.model_validate( + load_yaml_config( + _write( + tmp_path, + """ +analysis: + code_timeout: 120 +""", + ) + ) + ) + assert cfg.analysis.model is None + assert cfg.analysis.code_timeout == 120.0 + + +def test_analysis_model_explicit_overrides_qa(tmp_path): + """An explicit ``analysis.model`` in YAML wins over the qa fallback.""" + cfg = AppConfig.model_validate( + load_yaml_config( + _write( + tmp_path, + """ +qa: + model: + name: qa-model + provider: openai +analysis: + model: + name: analysis-model + provider: ollama +""", + ) + ) + ) + assert cfg.qa.model.name == "qa-model" + assert cfg.analysis.model is not None + assert cfg.analysis.model.name == "analysis-model" + resolved = cfg.analysis.model or cfg.qa.model + assert resolved.name == "analysis-model" diff --git a/tests/test_picture_in_context.py b/tests/test_picture_in_context.py index fefa1d9a..9c7e2cf1 100644 --- a/tests/test_picture_in_context.py +++ b/tests/test_picture_in_context.py @@ -761,3 +761,80 @@ async def test_search_tool_returns_plain_string_when_no_pictures(): assert isinstance(result, str) assert "rank 1" in result + + +@pytest.mark.asyncio +async def test_skill_search_tool_uses_skill_model_vision_flag(tmp_path): + """The skill-level ``search`` tool gates picture attachment on the model + passed to ``create_skill_tools`` (per-skill driving model), not on + ``config.qa.model.vision``. Verifies the per-skill plumbing by setting + qa.model.vision=False and analysis.model.vision=True simultaneously.""" + from haiku.rag.client import HaikuRAG + from haiku.rag.skills._deps import RAGRunDeps + from haiku.rag.skills._tools import create_skill_tools + from haiku.rag.skills.rag import RAGState + + picture_result = SearchResult( + content="A figure", + score=1.0, + chunk_id="chunk-1", + document_id="doc-1", + doc_item_refs=["#/pictures/0"], + labels=["picture"], + image_data={"#/pictures/0": PICTURE_B64}, + ) + + from haiku.rag.config.models import ModelConfig + + config = AppConfig() + config.qa.model.vision = False + # Explicitly set analysis.model (it defaults to None = inherit from qa). + config.analysis.model = ModelConfig( + provider="openai", name="vision-model", vision=True + ) + + async with HaikuRAG(tmp_path / "db.lancedb", create=True) as rag: + rag.search = AsyncMock(return_value=[picture_result]) # type: ignore[method-assign] + rag.expand_context = AsyncMock(return_value=[picture_result]) # type: ignore[method-assign] + + # rag skill: should NOT attach binaries (qa.model.vision is False) + rag_tools = create_skill_tools( + tmp_path / "db.lancedb", + config, + RAGState, + ["search"], + model=config.qa.model, + ) + deps = RAGRunDeps(state=RAGState(), rag=rag, emit=lambda _e: None) + ctx = RunContext( + deps=deps, + model=TestModel(), + usage=RunUsage(), + run_id="run-1", + ) + result_rag = await rag_tools["search"](ctx, "anything") + assert isinstance(result_rag, str), ( + "rag skill with qa.model.vision=False must return plain text" + ) + + # analysis skill: SHOULD attach binaries (analysis.model.vision is True) + analysis_tools = create_skill_tools( + tmp_path / "db.lancedb", + config, + RAGState, # state shape doesn't matter for this assertion + ["search"], + model=config.analysis.model or config.qa.model, + ) + deps2 = RAGRunDeps(state=RAGState(), rag=rag, emit=lambda _e: None) + ctx2 = RunContext( + deps=deps2, + model=TestModel(), + usage=RunUsage(), + run_id="run-2", + ) + result_analysis = await analysis_tools["search"](ctx2, "anything") + assert isinstance(result_analysis, ToolReturn), ( + "analysis skill with analysis.model.vision=True must wrap binaries" + ) + assert result_analysis.content is not None + assert any(isinstance(p, BinaryContent) for p in result_analysis.content) diff --git a/tests/test_skill_tools.py b/tests/test_skill_tools.py index cfe6797b..8364d8ed 100644 --- a/tests/test_skill_tools.py +++ b/tests/test_skill_tools.py @@ -76,6 +76,7 @@ def _build_search_tool(config: AppConfig): config=config, state_type=RAGState, tool_names=["search"], + model=config.qa.model, ) return tools["search"]