diff --git a/README.md b/README.md index 971e7bfe..eeefe6a5 100644 --- a/README.md +++ b/README.md @@ -87,7 +87,9 @@ The public tool LLMs see is called `llm_worker_call`, which maps its parameters Think of `llm_worker_call` as "delegate this subtask to a separate LLM worker with its own context and attachments," backed by the safety checks above. -This enforces allowlists, file size/type restrictions, and attachment limits. It also supports template locking (force all calls to use a specific vetted template) and structured outputs via `expect_json=True`. Only set `expect_json=True` if the target template defines `schema_object`; otherwise TemplateCall will error. When the child template omits its own `model`, TemplateCall automatically inherits the caller's model so sub-calls use the same provider/config by default. +This enforces allowlists, file size/type restrictions, and attachment limits. It also supports template locking (force all calls to use a specific vetted template) and structured outputs via `expect_json=True`. Only set `expect_json=True` if the target template defines `schema_object`; otherwise TemplateCall will error. + +Model selection is simple: TemplateCall uses the target template's `model` when present, otherwise it falls back to the global default model configured in `llm`. There is no per-toolbox default model parameter. ## Why TemplateCall? diff --git a/docs/templatecall.md b/docs/templatecall.md index 3548a91c..6259f980 100644 --- a/docs/templatecall.md +++ b/docs/templatecall.md @@ -9,7 +9,7 @@ This document explains why `TemplateCall` exists, how it fits into the `llm-do` - attachment validation (count, size, suffix) - fragment handling for extra context - structured outputs via `expect_json` + `schema_object` - - model inheritance and security toggles + - model fallback and tool security toggles - **LLMs** only see a generic tool named `llm_worker_call` that means "call a named LLM worker (preconfigured template) with its own context and attachments." The model chooses a `worker_name`, provides `input`, optional `attachments` and `extra_context`, and (optionally) `params`/`expect_json`. The worker's prompt, model, and allowed tools are defined elsewhere; the caller only passes arguments. ## What TemplateCall Does @@ -20,8 +20,8 @@ This document explains why `TemplateCall` exists, how it fits into the `llm-do` - **Attachment validation:** File count, size, and suffix restrictions enforced before passing to the LLM - **Fragment support:** Pass text snippets (procedures, rubrics, extracted sections) as additional context - **Structured outputs:** Optional JSON parsing and normalization via `expect_json=True` (only when the template defines `schema_object`). -- **Automatic model inheritance:** If the child template omits its own `model`, TemplateCall reuses the model currently running the parent template/tool call. -- **Security defaults:** Inline Python functions disabled by default (opt-in override available) +- **Model selection:** Use the template's `model` when present; otherwise fall back to the global default model configured in `llm`. +- **Security defaults:** Inline Python functions embedded in templates are ignored. Example configuration: @@ -65,6 +65,21 @@ LLM-facing tool surface (wired to the same implementation): `llm_worker_call` maps parameters like this: `worker_name` → `template`, `extra_context` → `fragments`, while `attachments`, `params`, and `expect_json` pass through with the same validation rules. (TODO: Consider adding additional tool aliases such as `delegate_task` or `call_subtask` if certain models respond better to alternate names.) +This enforces allowlists, file size/type restrictions, and attachment limits. It also supports template locking (force all calls to use a specific vetted template) and structured outputs via `expect_json=True`. Only set `expect_json=True` if the target template defines `schema_object`; otherwise TemplateCall will error. + +### Model selection + +TemplateCall resolves the model in two steps: + +1. The `model:` field on the target template +2. The global default model returned by `llm.get_default_model()` + +There is no per-TemplateCall default model parameter. + +### Inline functions are ignored + +`TemplateCall` skips inline `functions:` blocks defined inside templates. Register Python tools/toolboxes normally and reference them via `tools:` entries instead of embedding code inside YAML. + ## Why Recursion Matters When a template can call another template, the template language becomes recursively closed. This is useful for a couple of reasons: @@ -166,8 +181,8 @@ The `TemplateCall` toolbox is implemented in `llm_do/tools_template_call.py`. Ke - Template paths support `pkg:` prefix for package-bundled templates and filesystem paths for user templates - Attachment validation happens before invoking `llm` (fail fast if files are too large or have wrong extensions) - `expect_json=True` attempts to parse the response as JSON (only allowed when the template defines `schema_object`) and returns a normalized structure -- If the child template does not declare a `model`, TemplateCall automatically inherits the parent template's model for the sub-call. -- Inline Python functions in sub-templates are ignored by default (security); override via `allow_functions=True` if needed +- Model selection follows two steps: (1) the template's explicit `model` value, or (2) the global default model configured in `llm`. +- Inline Python `functions:` blocks in sub-templates are ignored; reference Python toolboxes via `tools:` instead. - Fragment files are read and passed as text to the template (useful for procedures, rubrics, or context snippets) - From the LLM's point of view, all of this is exposed as the single `llm_worker_call` tool. The model only decides which `worker_name` to call, what `input` to send, which files to attach, and any extra snippets of context. diff --git a/examples/pitchdeck_eval/templates/pitchdeck-orchestrator.yaml b/examples/pitchdeck_eval/templates/pitchdeck-orchestrator.yaml index 389d5462..8e100b5e 100644 --- a/examples/pitchdeck_eval/templates/pitchdeck-orchestrator.yaml +++ b/examples/pitchdeck_eval/templates/pitchdeck-orchestrator.yaml @@ -17,33 +17,6 @@ tools: "max_attachments": 1, "max_bytes": 25000000 }) -functions: | - import json - import re - - def slugify(text: str) -> str: - slug = re.sub(r"[^a-z0-9]+", "-", text.lower()).strip("-") - return slug or "deck" - - def render_markdown(payload: dict) -> str: - lines = [ - f"# {payload['deck_id']}", - "", - f"**Slug:** {payload['file_slug']}", - f"**Verdict:** {payload['verdict'].upper()}", - "", - "## Summary", - payload['summary'], - "", - "## Scores", - ] - for dimension, data in payload["scores"].items(): - lines.append(f"- **{dimension}**: {data['score']} – {data['evidence']}") - if payload["red_flags"]: - lines.extend(["", "## Red flags"]) - for flag in payload["red_flags"]: - lines.append(f"- {flag}") - return "\n".join(lines) prompt: | 1. Use Files_pipeline_ro_list("*.pdf") to enumerate decks. 2. Load the evaluation procedure if `pipeline/PROCEDURE.md` exists using @@ -54,7 +27,15 @@ prompt: | * attachments: a list containing the absolute path to the PDF * extra_context: include the procedure text if available * expect_json: true - - Parse the JSON response and convert it to Markdown via render_markdown. - - Write the Markdown to Files_evaluations_out_write_text using the slugified - filename `.md`. + - Parse the JSON response and convert it to Markdown with this structure: + * Title: "# {deck_id}" + * Slug line: "**Slug:** {file_slug}" where `file_slug` is the lowercase + filename with non-alphanumerics replaced by hyphens (fallback: "deck") + * Verdict line: "**Verdict:** {verdict uppercased}" + * "## Summary" heading followed by the summary text + * "## Scores" heading with bullet points for each score: + "- **{dimension}**: {score} – {evidence}" + * If red_flags is non-empty, add a "## Red flags" heading and bullet list + of the flags + - Write the Markdown to Files_evaluations_out_write_text using `.md`. 4. When finished, report which decks were evaluated and where the outputs live. diff --git a/llm_do/tools_template_call.py b/llm_do/tools_template_call.py index d16201ea..efe396d2 100644 --- a/llm_do/tools_template_call.py +++ b/llm_do/tools_template_call.py @@ -7,7 +7,6 @@ import json import os import sys -import textwrap from functools import lru_cache from importlib import resources from pathlib import Path @@ -15,7 +14,6 @@ import llm from llm.templates import Template -from llm.models import Response as _LLMResponse, AsyncResponse as _LLMAsyncResponse from pydantic import BaseModel, ConfigDict, Field, field_validator @@ -28,30 +26,12 @@ def _llm_cli_module(): return cli_module -def _current_chain_model_id() -> Optional[str]: - """Attempt to detect the model used for the current conversation/tool call.""" - frame = inspect.currentframe() - try: - while frame: - self_obj = frame.f_locals.get("self") - if isinstance(self_obj, (_LLMResponse, _LLMAsyncResponse)): - model = getattr(self_obj, "model", None) - if model is not None: - return getattr(model, "model_id", None) - frame = frame.f_back - finally: - del frame - return None - - class TemplateCallConfig(BaseModel): allow_templates: List[str] = Field(default_factory=list) allowed_suffixes: List[str] = Field(default_factory=list) max_attachments: int = 4 max_bytes: int = 10_000_000 - ignore_functions: bool = True lock_template: Optional[str] = None - default_model: Optional[str] = None debug: bool = False model_config = ConfigDict(arbitrary_types_allowed=True) @@ -89,9 +69,7 @@ def __init__( allowed_suffixes: Optional[List[str]] = None, max_attachments: Optional[int] = None, max_bytes: Optional[int] = None, - ignore_functions: Optional[bool] = None, lock_template: Optional[str] = None, - default_model: Optional[str] = None, debug: Optional[bool] = None, ): if config is not None and not isinstance(config, dict): @@ -105,12 +83,8 @@ def __init__( options["max_attachments"] = max_attachments if max_bytes is not None: options["max_bytes"] = max_bytes - if ignore_functions is not None: - options["ignore_functions"] = ignore_functions if lock_template is not None: options["lock_template"] = lock_template - if default_model is not None: - options["default_model"] = default_model if debug is not None: options["debug"] = debug @@ -164,13 +138,7 @@ def run( tool_instances = self._instantiate_tools(tmpl) - inherited_model = _current_chain_model_id() - model_name = ( - tmpl.model - or self.config.default_model - or inherited_model - or llm.get_default_model() - ) + model_name = tmpl.model or llm.get_default_model() model = llm.get_model(model_name) if self.config.debug: @@ -289,7 +257,6 @@ def _load_template(self, name: str) -> Template: raise LoadTemplateError(f"Template not found in package: {relative}") content = resource.read_text(encoding="utf-8") template = parse_template(name, content) - template._functions_is_trusted = True return template # Resolve relative paths to absolute path = Path(name).resolve() @@ -297,7 +264,6 @@ def _load_template(self, name: str) -> Template: raise LoadTemplateError(f"Template not found: {name}") content = path.read_text(encoding="utf-8") template = parse_template(str(path), content) - template._functions_is_trusted = True return template def _resolve_attachments(self, attachment_paths: List[str]) -> List[llm.Attachment]: @@ -321,9 +287,7 @@ def _resolve_attachments(self, attachment_paths: List[str]) -> List[llm.Attachme def _instantiate_tools(self, template: Template): cli = _llm_cli_module() - if not template.tools and ( - self.config.ignore_functions or not template.functions - ): + if not template.tools: return None tools = [] registered = llm.get_tools() @@ -342,10 +306,6 @@ def _instantiate_tools(self, template: Template): if spec not in registered: raise ValueError(f"Unknown tool '{spec}' in template") tools.append(registered[spec]) - if not self.config.ignore_functions and template.functions: - if not template._functions_is_trusted: - raise ValueError("Template functions are not trusted") - tools.extend(cli._tools_from_code(textwrap.dedent(template.functions))) return tools diff --git a/tests/test_template_call_tool.py b/tests/test_template_call_tool.py index 112c6e2f..342bd6ad 100644 --- a/tests/test_template_call_tool.py +++ b/tests/test_template_call_tool.py @@ -1,7 +1,6 @@ from __future__ import annotations import json -import os import pytest from llm_do.tools_template_call import TemplateCall @@ -62,36 +61,70 @@ def test_expect_json_without_schema_raises(template_without_schema): tool.run(template=str(template_without_schema), expect_json=True) -def test_template_call_infers_parent_model( - monkeypatch, template_without_model, sample_attachment -): +def test_template_model_wins(monkeypatch, tmp_path, sample_attachment): captured = {} class CaptureModel: - def __init__(self): - self.calls = [] - def prompt(self, prompt_text: str, **kwargs): - self.calls.append((prompt_text, kwargs)) class _Dummy: def text(self): return "{}" return _Dummy() - model_instance = CaptureModel() + template = tmp_path / "with_model.yaml" + template.write_text( + """ +model: template-model +prompt: | + respond in json +schema_object: + type: object + properties: + message: + type: string + required: [message] + """.strip() + ) def fake_get_model(name): captured["model_name"] = name - return model_instance + return CaptureModel() monkeypatch.setattr("llm.get_model", fake_get_model) - monkeypatch.setattr("llm.get_default_model", lambda: "default-model") - monkeypatch.setattr( - "llm_do.tools_template_call._current_chain_model_id", - lambda: "parent-model", + monkeypatch.setattr("llm.get_default_model", lambda: "fallback-model") + + tool = TemplateCall( + allow_templates=[str(template)], + allowed_suffixes=[".txt"], + max_attachments=1, + ) + tool.run( + template=str(template), + attachments=[str(sample_attachment)], ) + assert captured["model_name"] == "template-model" + + +def test_template_uses_global_default(monkeypatch, template_without_model, sample_attachment): + captured = {} + + class CaptureModel: + def prompt(self, prompt_text: str, **kwargs): + class _Dummy: + def text(self): + return "{}" + + return _Dummy() + + def fake_get_model(name): + captured["model_name"] = name + return CaptureModel() + + monkeypatch.setattr("llm.get_model", fake_get_model) + monkeypatch.setattr("llm.get_default_model", lambda: "global-default") + tool = TemplateCall( allow_templates=[str(template_without_model)], allowed_suffixes=[".txt"], @@ -101,7 +134,73 @@ def fake_get_model(name): template=str(template_without_model), attachments=[str(sample_attachment)], ) - assert captured["model_name"] == "parent-model" + + assert captured["model_name"] == "global-default" + + +def test_functions_blocks_are_ignored(monkeypatch, tmp_path): + template = tmp_path / "with_functions.yaml" + template.write_text( + """ +model: dummy +tools: + - helper_tool +functions: | + def unused(): + return "should not run" +prompt: | + respond in json +schema_object: + type: object + properties: + message: + type: string + required: [message] + """.strip() + ) + + tool_instance = object() + + monkeypatch.setattr("llm.get_tools", lambda: {"helper_tool": tool_instance}) + monkeypatch.setattr("llm.get_default_model", lambda: "dummy") + + import llm.cli as cli + + def explode_tools_from_code(code): + raise AssertionError("inline functions should be ignored") + + monkeypatch.setattr(cli, "_tools_from_code", explode_tools_from_code) + + class CaptureModel: + def __init__(self): + self.calls = [] + + def prompt(self, prompt_text: str, **kwargs): + self.calls.append(kwargs) + + class _Dummy: + def text(self): + return "{}" + + return _Dummy() + + model_instance = CaptureModel() + + def fake_get_model(name): + return model_instance + + monkeypatch.setattr("llm.get_model", fake_get_model) + + tool = TemplateCall( + allow_templates=[str(template)], + allowed_suffixes=[".txt"], + max_attachments=1, + ) + + tool.run(template=str(template)) + + assert model_instance.calls + assert model_instance.calls[0]["tools"] == [tool_instance] def test_debug_mode_via_env_variable(