analysis.model inherits qa.model when unset; per-skill vision gate
This commit is contained in:
parent
52e93d08eb
commit
74dea65f21
9 changed files with 188 additions and 18 deletions
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
||||
|
|
|
|||
|
|
@ -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()
|
||||
|
|
|
|||
|
|
@ -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"
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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"]
|
||||
|
||||
|
|
|
|||
Loading…
Reference in a new issue