diff --git a/docs/reference/advanced/managed-variables/configuration-reference.md b/docs/reference/advanced/managed-variables/configuration-reference.md index 9a95b8262..14db73752 100644 --- a/docs/reference/advanced/managed-variables/configuration-reference.md +++ b/docs/reference/advanced/managed-variables/configuration-reference.md @@ -132,6 +132,31 @@ def test_premium_config_handling(): # Back to normal after context exits ``` +#### Overrides and composition + +Overrides take precedence over both the provider value and the code default, but they are treated as the user's literal choice. Two consequences worth knowing: + +- **`@{ref}@` composition is skipped on overrides.** Overriding with `'hi @{user}@'` produces the literal string — `@{user}@` is *not* expanded. Composition is for resolving fragments out of the configured graph; an override sits outside that graph. +- **Template rendering still happens for `TemplateVariable` overrides**, as long as the override value is JSON-serializable. Overriding with `'Hi {{name}}'` and calling `get(Inputs(name='Alice'))` returns `'Hi Alice'`. An override that *isn't* JSON-serializable (e.g. an arbitrary Python object) skips the render pass and comes back exactly as passed — useful for `Variable[SomeClass]` where the value is a typed Python object rather than a template string. + +```python +import logfire + +logfire.configure() + +logfire.var('user', type=str, default='Alice') +greeting = logfire.var('greeting', type=str, default='Hello, @{user}@!') + +# Without an override: composition expands @{user}@. +print(greeting.get().value) +#> Hello, Alice! + +# Override is literal — @{user}@ is *not* expanded. +with greeting.override('Hi @{user}@'): + print(greeting.get().value) + #> Hi @{user}@ +``` + ### Dynamic Override Functions Override with a function that computes the value based on context: diff --git a/docs/reference/advanced/managed-variables/templates-and-composition.md b/docs/reference/advanced/managed-variables/templates-and-composition.md index 188f2a23c..d77378bbd 100644 --- a/docs/reference/advanced/managed-variables/templates-and-composition.md +++ b/docs/reference/advanced/managed-variables/templates-and-composition.md @@ -150,25 +150,33 @@ When `safety_rules` is updated in the Logfire UI, all variables that reference ` ### Composition Control Flow -The `@{}@` syntax supports a small Handlebars-compatible subset for composing variables. It supports simple references, dotted field reads, and block helpers that branch or iterate over a top-level referenced variable: +The `@{}@` syntax runs through the full Handlebars engine (just with `@{` / `}@` as the delimiter pair instead of the default `{{` / `}}`), so any expression form that works in Handlebars also works here — simple references, dotted field reads, block helpers, and helper sub-expressions: | Syntax | Description | |--------|-------------| | `@{variable_name}@` | Insert a variable's value | | `@{variable.field}@` | Access a nested field | | `@{#if variable}@...@{else}@...@{/if}@` | Conditional on whether a variable is set | +| `@{#if user.active}@...@{/if}@` | Conditional on a dotted field | | `@{#each items}@...@{/each}@` | Iterate over a list variable | - -Block helper conditions and iterables must be top-level variable names. Use `@{#if user}@...@{user.active}@...@{/if}@` rather than `@{#if user.active}@`. +| `@{#each items}@@{../top}@@{/each}@` | Access an outer-scope value from inside a block | ### Composition Tracking Every `@{ref}@` expansion is recorded in the resolution result. You can inspect which variables were composed and their values: -```python skip="true" -with agent_prompt.get() as resolved: +```python +import logfire + +logfire.configure() + +logfire.var('city', type=str, default='Paris') +report = logfire.var('report', type=str, default='Weather in @{city}@: sunny.') + +with report.get() as resolved: for ref in resolved.composed_from: - print(f" {ref.name}: version={ref.version}, label={ref.label}") + print(f'{ref.name}={ref.value!r} reason={ref.reason}') + #> city='Paris' reason=code_default ``` These composition details are also recorded as span attributes, so you can see the full composition chain in your Logfire traces. @@ -177,7 +185,7 @@ These composition details are also recorded as span attributes, so you can see t Template variables and composition work together. A common pattern is to compose reusable fragments via `@{ref}@` and render runtime inputs via `{{}}`: -```python skip="true" +```python from pydantic import BaseModel import logfire @@ -191,11 +199,7 @@ class ChatInputs(BaseModel): # Reusable fragment (no template inputs) -tone_instructions = logfire.var( - 'tone_instructions', - type=str, - default='Be friendly and concise.', -) +logfire.var('tone_instructions', type=str, default='Be friendly and concise.') # Template variable that composes the fragment and renders inputs chat_prompt = logfire.template_var( @@ -208,9 +212,73 @@ chat_prompt = logfire.template_var( # Resolution: compose @{tone_instructions}@ first, then render {{user_name}} and {{language}} with chat_prompt.get(ChatInputs(user_name='Alice', language='French')) as resolved: print(resolved.value) - # "You are helping Alice. Respond in French. Be friendly and concise." + #> You are helping Alice. Respond in French. Be friendly and concise. +``` + +### Recursive Resolution + +!!! warning "Different from plain Handlebars" + Standard Handlebars expressions like `{{greeting}}` perform a **one-shot string substitution**: whatever string `greeting` resolves to appears verbatim in the output. If that string happens to contain `{{name}}`, the inner `{{name}}` is *not* re-evaluated — it ends up in the output as the literal text `{{name}}`. + + `@{...}@` composition does the opposite: when the SDK substitutes a referenced variable, it first **fully resolves** that variable — including expanding any `@{...}@` references *inside* it — before splicing the result in. + +Concretely, composition walks the reference graph at resolution time. A tree like `parent → @{middle}@ → @{leaf}@` resolves leaf-first, builds `middle`, then substitutes the result into `parent`: + +```python +import logfire + +logfire.configure() + +logfire.var('leaf', type=str, default='LEAF') +logfire.var('middle', type=str, default='middle wraps @{leaf}@') +parent = logfire.var('parent', type=str, default='top: @{middle}@') + +with parent.get() as resolved: + print(resolved.value) + #> top: middle wraps LEAF + # composed_from mirrors the tree: + print(f'{resolved.composed_from[0].name} -> {resolved.composed_from[0].composed_from[0].name}') + #> middle -> leaf +``` + +Contrast with plain Handlebars rendering, where `{{...}}` only substitutes — no graph walk, no re-rendering of values that happen to look template-like: + +```python +from pydantic_handlebars import render + +print(render('{{greeting}}', {'greeting': 'Hello, {{name}}!', 'name': 'Alice'})) +#> Hello, {{name}}! ``` -### Cycle Detection +### Cycle and depth guards -The system detects circular references during validation. If variable A references `@{B}@` and variable B references `@{A}@`, `logfire.variables_validate()` reports the cycle, and `logfire.variables_push(strict=True)` fails instead of applying the invalid configuration. This prevents infinite loops during resolution. +Because resolution walks an arbitrary graph, two failure modes need explicit handling: cycles (`A → @{B}@`, `B → @{A}@`) and deep chains. Both are caught at two layers: + +- **Push / sync time** — `logfire.variables_validate()` reports reference errors and cycles; `logfire.variables_push(strict=True)` fails instead of applying an invalid configuration. The walk covers the *full* reachable graph (local code defaults and server-stored label values), so a cycle whose midpoint is a server-only variable is still detected. This is the loud-by-default path. +- **Runtime** — if an invalid composition slips through (e.g. a server value changed between validation and the next resolve), `Variable.get()` catches the cycle (via a visited-set) or depth overflow (`MAX_COMPOSITION_DEPTH = 20`) and falls back to the variable's *code default* with a `RuntimeWarning`. The exception is recorded on `ResolvedVariable.exception` and the resolution reason becomes `'other_error'` so callers can detect and react. The same fallback applies when a `@{ref}@` points at a variable that doesn't exist at runtime — this differs from a missing `{{field}}` (Handlebars' empty-string substitution); composition treats unresolvable references as a real failure. + +```python +import warnings + +import logfire +from logfire.variables import VariableCompositionError + +logfire.configure() + +# A pair of variables that reference each other — push-time validation +# would catch this; we register them here just to show what the runtime +# guard does when it does have to step in. +left = logfire.var('cycle_left', type=str, default='@{cycle_right}@') +logfire.var('cycle_right', type=str, default='@{cycle_left}@') + +with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter('always') + with left.get() as resolved: + # `resolved.reason` is `'other_error'` because composition failed, + # and `resolved.exception` is a `VariableCompositionError` (or a + # subclass like `VariableCompositionCycleError` for cycles). + print(resolved.reason, isinstance(resolved.exception, VariableCompositionError)) + #> other_error True + print(any('composition failed' in str(w.message) for w in caught)) + #> True +``` diff --git a/logfire/_internal/main.py b/logfire/_internal/main.py index 59530a450..69b3eea6e 100644 --- a/logfire/_internal/main.py +++ b/logfire/_internal/main.py @@ -2622,6 +2622,17 @@ def var( return variable + @overload + def template_var( + self, + name: str, + *, + default: T, + inputs_type: type[InputsT], + description: str | None = None, + ) -> TemplateVariable[T, InputsT]: ... + + @overload def template_var( self, name: str, @@ -2630,6 +2641,16 @@ def template_var( default: T | ResolveFunction[T], inputs_type: type[InputsT], description: str | None = None, + ) -> TemplateVariable[T, InputsT]: ... + + def template_var( + self, + name: str, + *, + type: type[T] | None = None, + default: T | ResolveFunction[T], + inputs_type: type[InputsT], + description: str | None = None, ) -> TemplateVariable[T, InputsT]: """Define a managed template variable with integrated rendering. @@ -2652,7 +2673,6 @@ class PromptInputs(BaseModel): prompt = logfire.template_var( 'system_prompt', - type=str, default='Hello {{user_name}}', inputs_type=PromptInputs, ) @@ -2663,16 +2683,30 @@ class PromptInputs(BaseModel): Args: name: Unique identifier for the variable. - type: Expected type for validation and JSON schema generation. + type: Expected type for validation and JSON schema generation. Can be a primitive + type or a Pydantic model. If not provided, the type is inferred from `default`. + Required when `default` is a resolve function. default: Default value used when no remote configuration is found. - Can also be a callable with `targeting_key` and `attributes` parameters. + When `type` is not provided, the type is inferred from this value. + Can also be a callable with `targeting_key` and `attributes` parameters + (requires `type` to be set explicitly). inputs_type: The type (typically a Pydantic `BaseModel`) describing the expected template inputs. Used for type-safe `get(inputs)` calls and JSON schema generation. description: Optional human-readable description of what the variable controls. """ import re - from logfire.variables.variable import TemplateVariable + from logfire.variables.variable import TemplateVariable, is_resolve_function + + if type is None: + if is_resolve_function(default): + raise TypeError( + 'When `default` is a resolve function (callable with targeting_key and attributes parameters), ' + '`type` must be provided to specify the variable value type.' + ) + tp = cast(Type[T], default.__class__) # noqa UP006 + else: + tp = type if not re.match(r'^[a-zA-Z_][a-zA-Z0-9_]*$', name): raise ValueError( @@ -2692,7 +2726,7 @@ class PromptInputs(BaseModel): variable = TemplateVariable[T, InputsT]( name, - type=type, + type=tp, default=default, inputs_type=inputs_type, description=description, diff --git a/logfire/variables/_handlebars.py b/logfire/variables/_handlebars.py index 23ef4c4df..f2519f6e8 100644 --- a/logfire/variables/_handlebars.py +++ b/logfire/variables/_handlebars.py @@ -1,11 +1,12 @@ from __future__ import annotations -from collections.abc import Callable -from functools import cache +from functools import cache, lru_cache from typing import TYPE_CHECKING, Any if TYPE_CHECKING: - from pydantic_handlebars import HandlebarsEnvironment + from types import ModuleType + + from pydantic_handlebars import CompiledTemplate, HandlebarsEnvironment # The reference-syntax composition pass consumes ``@{...}@`` placeholders and @@ -21,7 +22,7 @@ class _FallbackHandlebarsError(Exception): try: from pydantic_handlebars import HandlebarsError as _ImportedHandlebarsError -except ImportError: # pragma: no cover +except ImportError: _ImportedHandlebarsError = _FallbackHandlebarsError HandlebarsError: type[Exception] = _ImportedHandlebarsError @@ -31,16 +32,36 @@ class HandlebarsDependencyError(ImportError): """Raised when a Handlebars feature is used without pydantic-handlebars installed.""" -def _dependency_error() -> HandlebarsDependencyError: - return HandlebarsDependencyError( - 'Handlebars template rendering requires the `pydantic-handlebars` package, ' - 'which is installed by the `logfire[variables]` extra.' - ) +@cache +def _pydantic_handlebars() -> ModuleType: + """Return the cached `pydantic_handlebars` module, or raise a helpful error. + + The import is cached so every accessor below — `get_environment`, + `compile_composition_template`, `extract_composition_dependencies`, + etc. — can grab attributes off the returned module without each one + repeating its own try/except dance. Failure surfaces once, here, with + the install hint. + """ + try: + import pydantic_handlebars + except ModuleNotFoundError as exc: + # Only reframe the error if it's *pydantic_handlebars itself* that's + # missing. A future version pulling in a new transitive dep that + # isn't installed would raise `ModuleNotFoundError` with a different + # `exc.name`, and the user wants to see *that* name in the message + # rather than a misleading "install pydantic-handlebars" hint. + if exc.name != 'pydantic_handlebars': + raise + raise HandlebarsDependencyError( + 'Handlebars template rendering requires the `pydantic-handlebars` package, ' + 'which is installed by the `logfire[variables]` extra.' + ) from exc + return pydantic_handlebars def ensure_handlebars_available() -> None: """Raise a helpful error if pydantic-handlebars is unavailable.""" - get_environment() + _pydantic_handlebars() @cache @@ -51,55 +72,65 @@ def get_environment() -> HandlebarsEnvironment: `{{...}}` runtime placeholders in the template as plain content; a subsequent render pass with the default delimiters consumes those. """ - try: - from pydantic_handlebars import HandlebarsEnvironment - except ModuleNotFoundError as exc: # pragma: no cover - if exc.name == 'pydantic_handlebars': - raise _dependency_error() from exc - raise - return HandlebarsEnvironment(open_delim=COMPOSITION_OPEN_DELIM, close_delim=COMPOSITION_CLOSE_DELIM) + return _pydantic_handlebars().HandlebarsEnvironment( + open_delim=COMPOSITION_OPEN_DELIM, close_delim=COMPOSITION_CLOSE_DELIM + ) @cache -def get_handlebars_renderer() -> tuple[type[str], Callable[..., str]]: - """Return pydantic-handlebars SafeString and module-level render function.""" - try: - from pydantic_handlebars import SafeString, render - except ModuleNotFoundError as exc: # pragma: no cover - if exc.name == 'pydantic_handlebars': - raise _dependency_error() from exc - raise - return SafeString, render +def get_runtime_environment() -> HandlebarsEnvironment: + """Return a cached default-delimiter `HandlebarsEnvironment` for `{{...}}` rendering. + Used by `TemplateVariable.get(inputs)` to render the post-composition + serialized value against the provided inputs. + """ + return _pydantic_handlebars().HandlebarsEnvironment() -def extract_composition_dependencies(template: str) -> set[str]: - """Return the top-level `@{name}@` references in *template*. - Delegates to `pydantic_handlebars.extract_dependencies` configured for - the composition delimiters, so block helpers / dotted paths / etc. are - handled AST-correctly. +@cache +def get_safe_string_cls() -> type[str]: + """Return `pydantic_handlebars.SafeString`. + + Context values are wrapped in it so HTML auto-escaping (off by default + but enableable) doesn't munge them. """ - try: - from pydantic_handlebars import extract_dependencies - except ModuleNotFoundError as exc: # pragma: no cover - if exc.name == 'pydantic_handlebars': - raise _dependency_error() from exc - raise - return extract_dependencies(template, open_delim=COMPOSITION_OPEN_DELIM, close_delim=COMPOSITION_CLOSE_DELIM) + return _pydantic_handlebars().SafeString -@cache -def _get_template_compatibility_checker() -> Callable[[list[str], dict[str, Any]], Any]: - """Return pydantic-handlebars schema compatibility checker, or raise a helpful error.""" - try: - from pydantic_handlebars import check_template_compatibility as _check_template_compatibility - except ModuleNotFoundError as exc: # pragma: no cover - if exc.name == 'pydantic_handlebars': - raise _dependency_error() from exc - raise - return _check_template_compatibility +@lru_cache(maxsize=1024) +def compile_composition_template(source: str) -> CompiledTemplate: + """Return a cached `CompiledTemplate` for *source* under composition delimiters. + + Managed-variable values are typically stable across many resolutions, so + caching the parsed program lets `Variable._resolve` skip the parse on + every `get()` call. 1024 is large enough for any realistic number of + distinct templates in a single process while staying bounded for + long-running workers. Same rationale for `compile_runtime_template`. + """ + return get_environment().compile(source) + + +@lru_cache(maxsize=1024) +def compile_runtime_template(source: str) -> CompiledTemplate: + """Return a cached `CompiledTemplate` for *source* under default `{{...}}` delimiters.""" + return get_runtime_environment().compile(source) + + +@lru_cache(maxsize=1024) +def extract_composition_dependencies(template: str) -> set[str]: + """Return the top-level `@{name}@` references in *template*. + + Cached because cycle / reference validation runs over the same template + strings multiple times per push or sync. The underlying delegation goes + to `pydantic_handlebars.extract_dependencies` configured for the + composition delimiters, so block helpers, dotted paths, and helper + sub-expressions are handled AST-correctly. + """ + return _pydantic_handlebars().extract_dependencies( + template, open_delim=COMPOSITION_OPEN_DELIM, close_delim=COMPOSITION_CLOSE_DELIM + ) def check_template_compatibility(templates: list[str], schema: dict[str, Any]) -> Any: """Run pydantic-handlebars schema compatibility checking.""" - return _get_template_compatibility_checker()(templates, schema) + return _pydantic_handlebars().check_template_compatibility(templates, schema) diff --git a/logfire/variables/abstract.py b/logfire/variables/abstract.py index bca885c5b..dc369b579 100644 --- a/logfire/variables/abstract.py +++ b/logfire/variables/abstract.py @@ -3,14 +3,21 @@ import json import warnings from abc import ABC, abstractmethod +from collections import deque from collections.abc import Callable, Mapping, Sequence from contextlib import ExitStack from dataclasses import dataclass, field from typing import TYPE_CHECKING, Any, Generic, Literal, TypeVar +from logfire.variables._handlebars import compile_runtime_template, get_safe_string_cls + SyncMode = Literal['merge', 'replace'] if TYPE_CHECKING: + # Pydantic is pulled in by the `[variables]` extra, not base logfire — + # so its imports stay TYPE_CHECKING + function-local. `import logfire` + # itself must work without pydantic installed (Pyodide regression + # guard, exercised by `pyodide_test/test.mjs`). from pydantic import TypeAdapter import logfire @@ -187,10 +194,7 @@ def render_serialized_string(serialized_json: str, inputs: Any) -> str: Returns: The rendered JSON string. """ - from logfire.variables._handlebars import get_handlebars_renderer - - safe_string_cls, render_fn = get_handlebars_renderer() - + safe_string_cls = get_safe_string_cls() context = _inputs_to_context(inputs) # Wrap all string values in SafeString to disable HTML escaping. @@ -202,7 +206,7 @@ def render_serialized_string(serialized_json: str, inputs: Any) -> str: # might contain JSON-special characters (e.g., double quotes) that # would make the resulting JSON invalid. decoded = json.loads(serialized_json) - rendered_value = _render_json_value(decoded, render_fn, context) + rendered_value = _render_json_value(decoded, compile_runtime_template, context) return json.dumps(rendered_value) @@ -222,19 +226,23 @@ def _wrap_safe_value(value: Any, safe_string_cls: type[str]) -> Any: return value -def _render_json_value(value: Any, hbs_render: Callable[..., str], context: dict[str, Any]) -> Any: +def _render_json_value(value: Any, compile_template: Callable[[str], Any], context: dict[str, Any]) -> Any: """Recursively render Handlebars templates in a decoded JSON value. Only string values are rendered; dicts and lists are walked recursively. + *compile_template* is the LRU-cached compile helper from + `_handlebars.compile_runtime_template` — passing it in (rather than + importing here) keeps the recursion cheap and makes the cache hit on + repeated identical sources. """ if isinstance(value, str): if '{{' not in value: return value - return hbs_render(value, context) + return compile_template(value).render(context) if isinstance(value, dict): - return {k: _render_json_value(v, hbs_render, context) for k, v in value.items()} # pyright: ignore[reportUnknownVariableType] + return {k: _render_json_value(v, compile_template, context) for k, v in value.items()} # pyright: ignore[reportUnknownVariableType] if isinstance(value, list): - return [_render_json_value(item, hbs_render, context) for item in value] # pyright: ignore[reportUnknownVariableType] + return [_render_json_value(item, compile_template, context) for item in value] # pyright: ignore[reportUnknownVariableType] # Numbers, booleans, None pass through unchanged return value @@ -536,8 +544,16 @@ def _check_reference_errors( ) -> list[str]: """Check for reference errors: non-existent refs and cycles. - Scans local variable defaults and server label values for @{references}@ - and validates that referenced variables exist and there are no cycles. + Walks the full composition graph starting from each locally-registered + variable, transitively following `@{ref}@` edges into server-only + variables — so a missing reference reachable only through a chain that + passes through a server-only node still surfaces, as does a cycle whose + midpoints are server-only. + + `VariablesConfig` is treated as self-contained for substitution: any + `@{name}@` whose `name` isn't in either the local registration set or + `server_config` is reported as a non-existent reference, the same way + a registration miss is. """ from logfire.variables.composition import find_references from logfire.variables.config import LabeledValue @@ -545,40 +561,59 @@ def _check_reference_errors( warnings_list: list[str] = [] - # Collect all known variable names (local + server) all_names: set[str] = {v.name for v in variables} | set(server_config.variables.keys()) + locals_by_name = {v.name: v for v in variables} - # Build a reference graph: variable_name -> set of referenced names - ref_graph: dict[str, set[str]] = {} + def _refs_of(name: str) -> set[str]: + """Collect refs from every serialized value reachable for *name*. - # Scan local variable defaults for references - for variable in variables: + That's the local code default (if registered locally) plus every + labeled server value plus the `latest_version`. Failures to + serialize the local default are tolerated — we want the walker to + keep going. + """ refs: set[str] = set() - if not is_resolve_function(variable.default): + local = locals_by_name.get(name) + if local is not None and not is_resolve_function(local.default): try: - serialized_default = variable.type_adapter.dump_json(variable.default).decode('utf-8') + serialized_default = local.type_adapter.dump_json(local.default).decode('utf-8') refs.update(find_references(serialized_default)) except Exception: pass - - # Also scan server label values for this variable - server_var = server_config.variables.get(variable.name) + server_var = server_config.variables.get(name) if server_var is not None: - for _, labeled_value in server_var.labels.items(): - if isinstance(labeled_value, LabeledValue): - refs.update(find_references(labeled_value.serialized_value)) + for labeled in server_var.labels.values(): + if isinstance(labeled, LabeledValue): + refs.update(find_references(labeled.serialized_value)) if server_var.latest_version is not None: refs.update(find_references(server_var.latest_version.serialized_value)) + return refs + # BFS the composition graph from every local variable in declaration + # order. Each node we visit contributes its outgoing edges to + # `ref_graph` and, if any point at an unknown name, a + # non-existent-reference warning. Visited names are gated on `seen` so + # a shared sub-tree is walked once. + ref_graph: dict[str, set[str]] = {} + seen: set[str] = set() + frontier: deque[str] = deque(v.name for v in variables) + while frontier: + current = frontier.popleft() + if current in seen: + continue + seen.add(current) + refs = _refs_of(current) if refs: - ref_graph[variable.name] = refs - - # Check for non-existent references - for ref_name in refs: - if ref_name not in all_names: - warnings_list.append(f"Variable '{variable.name}' references '@{{{ref_name}}}@' which does not exist.") - - # Check for cycles using DFS + ref_graph[current] = refs + for ref in refs: + if ref not in all_names: + warnings_list.append(f"Variable '{current}' references '@{{{ref}}}@' which does not exist.") + elif ref not in seen: + frontier.append(ref) + + # Cycle detection on the assembled graph. Because the graph includes + # nodes reached transitively through server-only variables, cycles + # whose midpoints are server-only are now caught too. def _detect_cycles(graph: dict[str, set[str]]) -> list[list[str]]: cycles: list[list[str]] = [] visited: set[str] = set() @@ -587,7 +622,6 @@ def _detect_cycles(graph: dict[str, set[str]]) -> list[list[str]]: def dfs(node: str) -> None: if node in in_stack: - # Found a cycle - extract it cycle_start = path.index(node) cycles.append(path[cycle_start:] + [node]) return diff --git a/logfire/variables/composition.py b/logfire/variables/composition.py index 294f08952..3ed71b0dc 100644 --- a/logfire/variables/composition.py +++ b/logfire/variables/composition.py @@ -1,9 +1,11 @@ """Variable composition: expand `@{variable_name}@` references in serialized values. This module provides pure functions for expanding variable references in serialized -JSON strings. References use the `@{variable_name}@` syntax and are expanded using -a Handlebars-compatible subset: simple references, dotted field reads, and block -helpers whose condition/iterable is a top-level referenced variable. +JSON strings. References use the `@{variable_name}@` syntax and are expanded by +running the value through `pydantic_handlebars` with the composition delimiter +pair, so the full Handlebars syntax is available: simple references, dotted +field reads, block helpers (including with dotted or sub-expression headers like +`@{#if user.active}@`), and helper sub-expressions. Meanwhile, any `{{runtime}}` placeholders are preserved untouched for later template rendering. @@ -32,11 +34,14 @@ 'has_references', ) -# Matches unescaped @{ (not preceded by \). Used as a cheap gate so we only -# parse strings that actually contain composition syntax. Real reference -# extraction goes through `pydantic_handlebars.extract_dependencies` so block -# helpers, dotted paths, and subexpressions are all handled AST-correctly. -_HAS_REFERENCE = re.compile(r'(?= 0.2.1` a run of N +# backslashes before `@{` contributes `N // 2` literal `\`s and lets parity +# decide whether the mustache renders. Doing that count in a regex would +# need a variable-width lookbehind we can't portably write on 3.10/3.11, and +# is unnecessary now that the renderer is the single source of truth. +_HAS_OPEN_DELIM = '@{' # Dotted-reference matcher used by the unresolved-reference protection # helpers — those need a textual hook so the literal `@{name.field}@` source @@ -84,8 +89,18 @@ class ComposedReference: def has_references(serialized_value: str) -> bool: - """Quick check for any unescaped `@{` in a serialized value.""" - return _HAS_REFERENCE.search(serialized_value) is not None + r"""Quick check for any `@{` in a serialized value. + + Returns `True` whenever the string contains the composition open + delimiter, regardless of preceding backslashes. The actual + escape-or-real decision is made by `pydantic_handlebars` at render time + — distinguishing an escaped `\@{x}@` from an unescaped `@{x}@` here + would require a variable-width lookbehind (the renderer counts + backslash parity to match Handlebars.js semantics) and is unnecessary: + `extract_composition_dependencies` returns an empty set for escaped-only + strings, and the renderer correctly leaves the literal text in place. + """ + return _HAS_OPEN_DELIM in serialized_value def expand_references( @@ -98,9 +113,10 @@ def expand_references( ) -> tuple[str, list[ComposedReference]]: """Expand `@{var}@` references in a serialized variable value. - Uses the Handlebars engine so that `@{}@` supports simple references, - dotted field reads, and block helpers whose condition/iterable is a - top-level referenced variable while preserving `{{runtime}}` placeholders + Uses the Handlebars engine so `@{}@` supports the full Handlebars + syntax — simple references, dotted field reads, block helpers (including + with dotted or sub-expression headers like `@{#if user.active}@`), and + helper sub-expressions — while preserving `{{runtime}}` placeholders untouched. Args: @@ -137,12 +153,11 @@ def expand_references( # render an invalid value. return serialized_value, composed - # Collect all unique base variable names referenced anywhere in the decoded value. + # Collect all unique base variable names referenced anywhere in the decoded + # value. If there are none we still walk the structure through `_render_value` + # — the value may contain only escape sequences (`\@{x}@` etc.) that need + # to be processed through the renderer to produce the literal output. all_ref_names = _collect_ref_names(decoded) - if not all_ref_names: - # No references at all — return unchanged (but still unescape \@{ → @{). - expanded = _unescape_serialized(serialized_value) - return expanded, composed # Resolve each unique variable name and recursively expand nested references. context: dict[str, Any] = {} @@ -334,15 +349,17 @@ def _render_value(value: Any, context: dict[str, Any], unresolved_names: set[str ``@{name}@`` text so the renderer preserves them in the output. Dotted accesses against unresolved names are pre-protected so they retain the full ``@{name.field}@`` source rather than rendering as empty. + Strings without any composition delimiter pass straight through; strings + that contain `@{` (escaped or not) route through the renderer, which + handles the backslash-parity escape rule. """ if isinstance(value, str): if not has_references(value): - # Unescape \@{ to @{ for non-reference strings. - return value.replace('\\@{', '@{') + return value from logfire.variables.reference_syntax import render_once protected_value, protected_refs = _protect_unresolved_dotted_refs(value, unresolved_names) - rendered = render_once(protected_value, context) if has_references(protected_value) else protected_value + rendered = render_once(protected_value, context) return _restore_unresolved_refs(rendered, protected_refs) if isinstance(value, dict): return { @@ -384,12 +401,3 @@ def _restore_unresolved_refs(value: str, protected_refs: dict[str, str]) -> str: for sentinel, ref in protected_refs.items(): value = value.replace(sentinel, ref) return value - - -def _unescape_serialized(serialized: str) -> str: - r"""Unescape `\@{` to `@{` in a JSON-serialized string. - - In JSON encoding, a literal backslash is `\\`, so `\@{` in user content - appears as `\\@{` in the serialized JSON. - """ - return serialized.replace('\\\\@{', '@{') diff --git a/logfire/variables/reference_syntax.py b/logfire/variables/reference_syntax.py index cc75b2a71..e630930fb 100644 --- a/logfire/variables/reference_syntax.py +++ b/logfire/variables/reference_syntax.py @@ -16,7 +16,7 @@ from typing import Any -from logfire.variables._handlebars import get_environment +from logfire.variables._handlebars import compile_composition_template def render_once(template: str, context: dict[str, Any]) -> str: @@ -25,5 +25,9 @@ def render_once(template: str, context: dict[str, Any]) -> str: `{{...}}` runtime placeholders in *template* are not touched — they are plain content under the configured delimiters. The escape sequence `\@{` produces a literal `@{` in the output. + + Compiled templates are cached + (`compile_composition_template` is an `lru_cache`) so resolving the + same managed-variable value repeatedly doesn't re-parse the source. """ - return get_environment().render(template, context) + return compile_composition_template(template).render(context) diff --git a/logfire/variables/variable.py b/logfire/variables/variable.py index 32e780b78..92fc94d70 100644 --- a/logfire/variables/variable.py +++ b/logfire/variables/variable.py @@ -19,7 +19,6 @@ ComposedReference, VariableCompositionError, expand_references, - has_references, ) if TYPE_CHECKING: @@ -182,6 +181,36 @@ def _deserialize(self, serialized_value: str) -> T_co | ValidationError | ValueE def override(self, value: T_co | ResolveFunction[T_co]) -> Generator[None]: """Context manager to temporarily override this variable's value. + Inside the `with` block, every `get()` call on this variable returns + *value* (or the result of calling it as a `ResolveFunction`) instead + of consulting the provider or code default. + + ## Composition is skipped + + Overrides do **not** participate in `@{ref}@` composition: *value* + is treated as the user's literal choice and is returned as-is, not + as a template to expand. If you override with the string + `'hi @{user}@'`, the `@{user}@` is *not* substituted — the result + is the literal string. Use composition by leaving the override off + and letting the default / provider value drive resolution. + + ## Template rendering still applies to TemplateVariable + + For `TemplateVariable.get(inputs)`, `{{...}}` rendering against + *inputs* runs on the override the same way it would on a + provider value — *as long as the override is JSON-serializable*. + For example, overriding with `'Hi {{name}}'` and calling + `get(Inputs(name='Alice'))` yields `'Hi Alice'`. + + ## Unserializable overrides come back verbatim + + If *value* can't be serialized through the variable's type adapter + (e.g. an arbitrary Python object on a `Variable[object]`), the + override is returned exactly as you passed it in, with no + serialize/deserialize round-trip and no template render pass. This + matches the "literal user choice" intent — a non-JSON Python value + is by definition not a template, so there's nothing to render. + Args: value: The value to use within this context, or a function that computes the value based on targeting_key and attributes. @@ -211,6 +240,21 @@ def _resolve( ) -> ResolvedVariable[T_co]: serialized_result: ResolvedVariable[str | None] | None = None try: + # Top-level context-override fast path: handled here, before + # `_lookup_serialized` even sees the name. Overrides do not + # participate in `@{ref}@` composition (their value is the user's + # literal choice), and the round-trip through dump_json / + # validate_json that `_lookup_serialized` would otherwise perform + # silently drops any value that isn't JSON-serializable. Restore + # the pre-#1951 behaviour: if the override serializes, take the + # render path; if it doesn't, return the typed Python value + # verbatim. + context_overrides = _VARIABLE_OVERRIDES.get() + if context_overrides is not None and self.name in context_overrides: + return self._resolve_context_override( + context_overrides[self.name], targeting_key, attributes, render_fn + ) + provider = self.logfire_instance.config.get_variable_provider() serialized_result = self._lookup_serialized( @@ -221,16 +265,6 @@ def _resolve( label=label, ) - # Context overrides skip composition expansion: the override is the - # user's literal choice, so we only optionally render (for - # TemplateVariable) and then deserialize. - if serialized_result.reason == 'context_override' and serialized_result.value is not None: - serialized = serialized_result.value - if render_fn is not None: - serialized = render_fn(serialized) - value = self.type_adapter.validate_json(serialized) - return ResolvedVariable(name=self.name, value=value, reason='context_override') - if serialized_result.value is None: return self._resolve_code_default( targeting_key, @@ -261,6 +295,38 @@ def _resolve( default = cast('T_co', None) return ResolvedVariable(name=self.name, value=default, exception=e, reason='other_error') + def _resolve_context_override( + self, + override_value: T_co | ResolveFunction[T_co], + targeting_key: str | None, + attributes: Mapping[str, Any] | None, + render_fn: Callable[[str], str] | None, + ) -> ResolvedVariable[T_co]: + """Return a resolution for the top-level context override. + + Overrides do not participate in composition. When the override value + serializes cleanly, run any provided `render_fn` (template rendering) + against the JSON form and revalidate so the user gets the same shape + a provider value would yield. When it doesn't serialize — common for + custom Python types, arbitrary objects, etc. — return the user's + value verbatim under `reason='context_override'`. Returning verbatim + is the legacy behaviour Devin / Alex flagged on #1951; the previous + implementation silently dropped these values back to the provider / + code default. + """ + if is_resolve_function(override_value): + resolved_value = cast('T_co', override_value(targeting_key, attributes)) + else: + resolved_value = cast('T_co', override_value) + try: + serialized = self.type_adapter.dump_json(resolved_value).decode('utf-8') + except (ValueError, TypeError, RuntimeError): + return ResolvedVariable(name=self.name, value=resolved_value, reason='context_override') + if render_fn is not None: + serialized = render_fn(serialized) + validated = self.type_adapter.validate_json(serialized) + return ResolvedVariable(name=self.name, value=validated, reason='context_override') + def _lookup_serialized( self, name: str, @@ -339,53 +405,59 @@ def _expand_and_deserialize( serialized_value = serialized_result.value composed: list[ComposedReference] = [] - # Expand @{references}@ if any are present - if has_references(serialized_value): - - def resolve_ref( - ref_name: str, - ) -> tuple[str | None, str | None, int | None, ResolutionReason]: - # Shares the lookup priority with `_resolve` so that composition - # respects overrides and registered code defaults rather than - # only consulting the provider. - ref_result = self._lookup_serialized( - ref_name, - provider=provider, - targeting_key=targeting_key, - attributes=attributes, - ) - return (ref_result.value, ref_result.label, ref_result.version, ref_result.reason) + # Always run through `expand_references`, even when no `@{ref}@` tags + # are present: it's also responsible for unescaping `\@{...}@` → + # `@{...}@`. Gating on `has_references` produced inconsistent + # observable behaviour where an escaped-only value (e.g. + # `r'\@{baz}@'`) kept its backslash, but the same escape combined + # with a real reference correctly produced the literal `@{baz}@`. + def resolve_ref( + ref_name: str, + ) -> tuple[str | None, str | None, int | None, ResolutionReason]: + # Shares the lookup priority with `_resolve` so that composition + # respects overrides and registered code defaults rather than + # only consulting the provider. + ref_result = self._lookup_serialized( + ref_name, + provider=provider, + targeting_key=targeting_key, + attributes=attributes, + ) + return (ref_result.value, ref_result.label, ref_result.version, ref_result.reason) - try: - serialized_value, composed = expand_references( - serialized_value, - self.name, - resolve_ref, - ) - if composition_error := _first_composition_error(composed): - return self._composition_failure( - exception=VariableCompositionError(composition_error), - targeting_key=targeting_key, - attributes=attributes, - serialized_result=serialized_result, - composed=composed, - ) - except VariableCompositionError as e: - return self._composition_failure( - exception=e, + try: + serialized_value, composed = expand_references( + serialized_value, + self.name, + resolve_ref, + ) + if composition_error := _first_composition_error(composed): + return self._fallback_to_default( + exception=VariableCompositionError(composition_error), + failure_stage='composition', targeting_key=targeting_key, attributes=attributes, serialized_result=serialized_result, composed=composed, ) + except VariableCompositionError as e: + return self._fallback_to_default( + exception=e, + failure_stage='composition', + targeting_key=targeting_key, + attributes=attributes, + serialized_result=serialized_result, + composed=composed, + ) # Apply render_fn (template rendering) if provided if render_fn is not None: try: serialized_value = render_fn(serialized_value) except (HandlebarsError, ValueError, TypeError) as e: - return self._composition_failure( + return self._fallback_to_default( exception=e, + failure_stage='template rendering', targeting_key=targeting_key, attributes=attributes, serialized_result=serialized_result, @@ -418,18 +490,26 @@ def resolve_ref( composed_from=composed, ) - def _composition_failure( + def _fallback_to_default( self, *, exception: Exception, + failure_stage: str, targeting_key: str | None, attributes: Mapping[str, Any] | None, serialized_result: ResolvedVariable[str | None], composed: list[ComposedReference], ) -> ResolvedVariable[T_co]: - """Fall back to the code default and warn after a composition/render failure.""" + """Fall back to the code default and warn after a composition or render failure. + + *failure_stage* identifies which step in the pipeline failed so the + warning text is accurate: composition (`@{ref}@` expansion) and + template rendering (`{{...}}` against `inputs`) reach this fallback + through different branches and shouldn't all surface as + "composition failed". + """ warnings.warn( - f"Variable '{self.name}' composition failed; falling back to code default: {exception}", + f"Variable '{self.name}' {failure_stage} failed; falling back to code default: {exception}", category=RuntimeWarning, stacklevel=2, ) diff --git a/pyproject.toml b/pyproject.toml index c3f4cf10b..87c898672 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -84,7 +84,7 @@ google-genai = ["opentelemetry-instrumentation-google-genai >= 0.4b0"] litellm = ["openinference-instrumentation-litellm >= 0"] dspy = ["openinference-instrumentation-dspy >= 0"] datasets = ["httpx>=0.27.2", "pydantic>=2", "pydantic-evals>=1.0.0"] -variables = ["pydantic>=2", "pydantic-handlebars>=0.2.0"] +variables = ["pydantic>=2", "pydantic-handlebars>=0.2.1"] [project.urls] Homepage = "https://pydantic.dev/logfire" @@ -201,7 +201,7 @@ dev = [ "pytest-examples>=0.0.18", "pytest-timeout>=2.4.0", "pytest-asyncio>=0.24.0", - "pydantic-handlebars>=0.2.0", + "pydantic-handlebars>=0.2.1", "claude-agent-sdk>=0", ] docs = [ diff --git a/tests/test_push_variables.py b/tests/test_push_variables.py index 30cb908be..602aacbbe 100644 --- a/tests/test_push_variables.py +++ b/tests/test_push_variables.py @@ -400,6 +400,80 @@ def test_compute_diff_reference_error_scan_skips_already_visited_nodes( assert diff.reference_errors == [] +def test_compute_diff_reference_errors_through_server_only_chain( + mock_logfire_instance: MockLogfire, +) -> None: + """Missing refs reached through server-only variables are surfaced. + + Reproduces the case from #1951's review: a local variable composes a + server-only fragment whose own value references a third name that + doesn't exist anywhere. The walker has to follow `@{ref}@` edges out + of server-only nodes — values are in `VariablesConfig`, the walker + just needs to follow them. + """ + foo2 = Variable[str]( + name='foo2', + default='@{foo1}@', + type=str, + logfire_instance=mock_logfire_instance, # type: ignore + ) + server_config = VariablesConfig( + variables={ + 'foo1': VariableConfig( + name='foo1', + json_schema={'type': 'string'}, + labels={}, + rollout=Rollout(labels={}), + overrides=[], + latest_version=LatestVersion(version=1, serialized_value='"foo1 references @{foo3}@"'), + ), + } + ) + + diff = _compute_diff([foo2], server_config) + + assert any("'foo1' references '@{foo3}@'" in warning for warning in diff.reference_errors) + + +def test_compute_diff_detects_cycle_through_server_only_chain( + mock_logfire_instance: MockLogfire, +) -> None: + """Cycles whose midpoints are server-only are detected.""" + foo = Variable[str]( + name='foo', + default='@{server_a}@', + type=str, + logfire_instance=mock_logfire_instance, # type: ignore + ) + # foo -> server_a -> server_b -> foo (cycle, server_a and server_b + # are server-only — no local registration). + server_config = VariablesConfig( + variables={ + 'server_a': VariableConfig( + name='server_a', + json_schema={'type': 'string'}, + labels={'production': LabeledValue(version=1, serialized_value='"@{server_b}@"')}, + rollout=Rollout(labels={'production': 1.0}), + overrides=[], + ), + 'server_b': VariableConfig( + name='server_b', + json_schema={'type': 'string'}, + labels={'production': LabeledValue(version=1, serialized_value='"@{foo}@"')}, + rollout=Rollout(labels={'production': 1.0}), + overrides=[], + ), + } + ) + + diff = _compute_diff([foo], server_config) + + assert any( + 'Reference cycle detected' in warning and 'foo' in warning and 'server_a' in warning and 'server_b' in warning + for warning in diff.reference_errors + ) + + def test_format_diff_creates() -> None: """Test diff formatting for creates.""" diff = VariableDiff( diff --git a/tests/test_variable_composition.py b/tests/test_variable_composition.py index 5c5ef0e0d..4e1dec7a3 100644 --- a/tests/test_variable_composition.py +++ b/tests/test_variable_composition.py @@ -292,10 +292,18 @@ def test_list_with_references(self): assert [ref.name for ref in composed] == ['greeting', 'name'] def test_keyword_block_references_are_ignored(self): - """Handlebars built-in names are not treated as variable references.""" + """Handlebars built-in names (`this`, helpers, `else`) aren't treated as variable references. + + `@{#if this}@yes@{/if}@` is a real Handlebars `if` block whose + condition is the current context (`this`). It evaluates normally — + with an empty context object (truthy in JS-style truthiness, which + pydantic-handlebars follows) the body renders as `yes`. The + important property under test is that no `composed` entries are + produced — `this` and `if` are not resolved as variable lookups. + """ expanded, composed = expand_references(json.dumps('@{#if this}@yes@{/if}@'), 'my_var', _make_resolve_fn({})) - assert json.loads(expanded) == '@{#if this}@yes@{/if}@' + assert json.loads(expanded) == 'yes' assert composed == [] def test_json_encoding_newlines(self): @@ -495,10 +503,17 @@ def test_referenced_html_entities_are_preserved(self): assert json.loads(expanded) == 'literal { and }' def test_referenced_escaped_reference_is_preserved(self): - r"""Escaped reference syntax inside referenced values keeps its backslash.""" + r"""Escaped reference syntax inside referenced values becomes the literal text post-render. + + Per `pydantic-handlebars >= 0.2.1` (and the Handlebars.js spec it + matches), the escape `\@{...}@` consumes the backslash and emits + the literal `@{...}@` in the output. The inner content is preserved + — just unescaped of its backslash — so callers can author "this + looks like a ref but render it literally" payloads. + """ resolve_fn = _make_resolve_fn({'ref': json.dumps(r'\@{not_a_ref}@')}) expanded, _ = expand_references('"@{ref}@"', 'my_var', resolve_fn) - assert json.loads(expanded) == r'\@{not_a_ref}@' + assert json.loads(expanded) == '@{not_a_ref}@' class TestExpandReferencesNativeHandlebarsSyntax: @@ -617,6 +632,65 @@ def test_simple_reference(self, config_kwargs: dict[str, Any]): assert result.composed_from[0].name == 'greeting' assert result.composed_from[0].value == 'Hello' + def test_escape_only_value_is_unescaped_consistently(self, config_kwargs: dict[str, Any]): + r"""Escape behaviour matches whether or not another real `@{ref}@` is present. + + Regression for #1951 r3288986490 — a value containing only an escaped + `\@{baz}@` used to keep its backslash, while the same escape combined + with a real reference produced literal `@{baz}@`. After dropping the + `has_references` short-circuit both go through the unescape path. + """ + config_kwargs['variables'] = LocalVariablesOptions(config=VariablesConfig(variables={})) + lf = logfire.configure(**config_kwargs) + + # No real refs — must still unescape. + bar2 = lf.var(name='bar2', default=r'\@{baz}@', type=str) + assert bar2.get().value == '@{baz}@' + + # Escape + real ref — both unescape (existing behaviour, asserted as a + # consistency anchor with the previous case). + baz = lf.var(name='baz', default='BAZ', type=str) + bar3 = lf.var(name='bar3', default=r'@{baz}@ and \@{baz}@', type=str) + assert bar3.get().value == 'BAZ and @{baz}@' + + # Used in the test_simple_reference style: silence unused-var warning + # by referencing `baz` once. + assert baz.get().value == 'BAZ' + + def test_backslash_run_parity_under_composition(self, config_kwargs: dict[str, Any]): + r"""Even-length backslash runs render the mustache; odd-length escape it. + + Regression for the bug exposed by pydantic-handlebars 0.2.1 — the + previous logfire-side `has_references` regex treated *any* preceding + backslash as the escape marker, so `\\@{x}@` (two backslashes) was + seen as "no refs" and rendered as-is. With 0.2.1's spec-compliant + renderer plus the simplified `'@{' in v` gate, both odd and even + runs route through the renderer and resolve per Handlebars.js rules: + + N backslashes contributes N // 2 literal `\` characters; parity + decides whether the mustache renders (even) or stays literal (odd). + """ + config_kwargs['variables'] = LocalVariablesOptions(config=VariablesConfig(variables={})) + lf = logfire.configure(**config_kwargs) + + lf.var(name='x', default='X', type=str) + + # 1 backslash → escape mustache, no literal backslash in output. + one = lf.var(name='one', default=r'\@{x}@', type=str) + assert one.get().value == '@{x}@' + + # 2 backslashes → one literal backslash, then mustache renders. + two = lf.var(name='two', default=r'\\@{x}@', type=str) + assert two.get().value == r'\X' + + # 3 backslashes → one literal backslash, then escape mustache. + three = lf.var(name='three', default=r'\\\@{x}@', type=str) + assert three.get().value == r'\@{x}@' + + # 4 backslashes → two literal backslashes, then mustache renders. + four = lf.var(name='four', default=r'\\\\@{x}@', type=str) + assert four.get().value == r'\\X' + def test_composition_exception_falls_back(self, config_kwargs: dict[str, Any], monkeypatch: pytest.MonkeyPatch): """Composition engine failures fall back to the code default.""" variables_config = _make_variables_config( diff --git a/tests/test_variable_templates.py b/tests/test_variable_templates.py index ed96d2935..adc24e488 100644 --- a/tests/test_variable_templates.py +++ b/tests/test_variable_templates.py @@ -99,7 +99,7 @@ def blocked_import(name, globals=None, locals=None, fromlist=(), level=0): def test_handlebars_import_helpers_are_memoized(monkeypatch: pytest.MonkeyPatch): """Successful pydantic-handlebars imports are cached after the first lookup.""" - renderer = _handlebars.get_handlebars_renderer() + module = _handlebars._pydantic_handlebars() schema = {'type': 'object', 'properties': {'name': {'type': 'string'}}} _handlebars.check_template_compatibility(['Hello {{name}}'], schema) @@ -112,10 +112,64 @@ def blocked_import(name: str, *args: Any, **kwargs: Any) -> Any: monkeypatch.setattr(builtins, '__import__', blocked_import) - assert _handlebars.get_handlebars_renderer() == renderer + # Re-entering the cached accessors should not re-trigger the import. + assert _handlebars._pydantic_handlebars() is module _handlebars.check_template_compatibility(['Hello {{name}}'], schema) +def test_missing_handlebars_raises_helpful_error(monkeypatch: pytest.MonkeyPatch): + """When `pydantic_handlebars` can't be imported, the SDK raises a guided error. + + Run in-process — same `__import__` blocking trick as + `test_import_logfire_without_pydantic_handlebars`, but here we wipe the + `_pydantic_handlebars` cache first so the import re-runs and hits the + dep-error branch. The subprocess version exercises the user-facing + `template_var` / `render_serialized_string` flows; this one gives the + coverage harness an in-process witness of the failure path. + """ + real_import = builtins.__import__ + + def blocked_import(name: str, *args: Any, **kwargs: Any) -> Any: + if name == 'pydantic_handlebars' or name.startswith('pydantic_handlebars.'): + raise ModuleNotFoundError(f'No module named {name!r}', name=name) + return real_import(name, *args, **kwargs) + + monkeypatch.setattr(builtins, '__import__', blocked_import) + _handlebars._pydantic_handlebars.cache_clear() + try: + with pytest.raises(_handlebars.HandlebarsDependencyError, match='pydantic-handlebars'): + _handlebars._pydantic_handlebars() + finally: + # Restore the real import so subsequent tests in the session see a + # populated cache. + _handlebars._pydantic_handlebars.cache_clear() + + +def test_unrelated_module_not_found_propagates(monkeypatch: pytest.MonkeyPatch): + """A `ModuleNotFoundError` for a *different* module isn't reframed. + + Simulates the case where a future `pydantic_handlebars` version pulls in + a new transitive dependency that the user hasn't installed. We want the + real error to surface so the user sees which package they actually need + — reframing it as "install pydantic-handlebars" would be misleading. + """ + real_import = builtins.__import__ + + def blocked_import(name: str, *args: Any, **kwargs: Any) -> Any: + if name == 'pydantic_handlebars': + raise ModuleNotFoundError("No module named 'some_other_dep'", name='some_other_dep') + return real_import(name, *args, **kwargs) + + monkeypatch.setattr(builtins, '__import__', blocked_import) + _handlebars._pydantic_handlebars.cache_clear() + try: + with pytest.raises(ModuleNotFoundError, match='some_other_dep') as exc_info: + _handlebars._pydantic_handlebars() + assert not isinstance(exc_info.value, _handlebars.HandlebarsDependencyError) + finally: + _handlebars._pydantic_handlebars.cache_clear() + + # ============================================================================= # VariableConfig.template_inputs_schema tests # ============================================================================= @@ -178,6 +232,31 @@ class Inputs(BaseModel): with pytest.raises(ValueError, match='Invalid variable name'): lf.template_var('not-valid', type=str, default='x', inputs_type=Inputs) + def test_type_inferred_from_default(self, config_kwargs: dict[str, Any]): + """template_var() infers `type` from a non-callable default, mirroring `var()`.""" + + class Inputs(BaseModel): + name: str + + lf = _make_lf(_simple_config('greeting', json.dumps('Hi {{name}}!')), config_kwargs) + var = lf.template_var('greeting', default='Hi {{name}}!', inputs_type=Inputs) + assert var.value_type is str + assert var.get(Inputs(name='Alice')).value == 'Hi Alice!' + + def test_type_required_for_resolve_function_default(self, config_kwargs: dict[str, Any]): + """template_var() with a callable default still requires an explicit `type=`.""" + + class Inputs(BaseModel): + name: str + + lf = logfire.configure(**config_kwargs) + + def make_default(targeting_key: str | None, attributes: Any) -> str: + return 'Hi {{name}}!' + + with pytest.raises(TypeError, match='resolve function'): + lf.template_var('greeting', default=make_default, inputs_type=Inputs) + def test_remote_render_error_records_exception(self, config_kwargs: dict[str, Any]): """Invalid remote templates fall back, warn, and record the render exception.""" @@ -187,7 +266,7 @@ class Inputs(BaseModel): lf = _make_lf(_simple_config('prompt', json.dumps('Hello {{#if name}}')), config_kwargs) var = lf.template_var('prompt', type=str, default='fallback', inputs_type=Inputs) - with pytest.warns(RuntimeWarning, match='composition failed'): + with pytest.warns(RuntimeWarning, match='template rendering failed'): resolved = var.get(Inputs(name='Alice')) assert resolved.value == 'fallback' @@ -460,11 +539,14 @@ class Inputs(BaseModel): ) # `model_construct` bypasses the constructor's validation so the override can hold - # the unrendered template; rendering then produces 'abc123', which fails the - # pattern constraint and exercises the outer error handler's code-default fallback. - bad_override = Config.model_construct(code='{{code}}') - with var.override(bad_override): - resolved = var.get(Inputs(code='abc123')) + # the unrendered template — `templated_config` is itself a *valid* template; it's + # only the rendered result (`abc123`) that violates the pattern constraint. + # Exercises the outer error handler's code-default fallback for inputs that + # produce a constraint-violating render. + templated_config = Config.model_construct(code='{{code}}') + invalid_inputs = Inputs(code='abc123') + with var.override(templated_config): + resolved = var.get(invalid_inputs) assert resolved.value == Config(code='OK') # falls back to the code default assert resolved.reason == 'other_error' diff --git a/tests/test_variables.py b/tests/test_variables.py index 230706e0e..9d51161fb 100644 --- a/tests/test_variables.py +++ b/tests/test_variables.py @@ -1882,6 +1882,29 @@ def test_override_nested(self, config_kwargs: dict[str, Any], variables_config: assert var.get().value == 'inner' assert var.get().value == 'outer' + def test_override_unserializable_value_returned_typed( + self, config_kwargs: dict[str, Any], variables_config: VariablesConfig + ): + """Top-level overrides return the user's typed Python value, even when it isn't JSON-serializable. + + Regression for #1951 r3287492856 / r3289439477 — the SDK previously + round-tripped overrides through `dump_json`/`validate_json`, silently + dropping unserializable values back to the provider / code default + and leaving callers unaware. Pre-#1951 behaviour was to return the + typed object verbatim; this restores that for the no-composition + case while still serializing when composition / template rendering + needs a string. + """ + config_kwargs['variables'] = LocalVariablesOptions(config=variables_config) + lf = logfire.configure(**config_kwargs) + + var = lf.var(name='string_var', default='default_value', type=object) + sentinel = object() + with var.override(sentinel): + result = var.get() + assert result.value is sentinel + assert result.reason == 'context_override' + def test_override_with_function(self, config_kwargs: dict[str, Any], variables_config: VariablesConfig): config_kwargs['variables'] = LocalVariablesOptions(config=variables_config) lf = logfire.configure(**config_kwargs) diff --git a/uv.lock b/uv.lock index 6d0f45f75..40d427a6d 100644 --- a/uv.lock +++ b/uv.lock @@ -1210,7 +1210,7 @@ name = "exceptiongroup" version = "1.3.1" source = { registry = "https://pypi.org/simple" } dependencies = [ - { name = "typing-extensions", marker = "python_full_version < '3.13'" }, + { name = "typing-extensions", marker = "python_full_version < '3.11'" }, ] sdist = { url = "https://files.pythonhosted.org/packages/50/79/66800aadf48771f6b62f7eb014e352e5d06856655206165d775e675a02c9/exceptiongroup-1.3.1.tar.gz", hash = "sha256:8b412432c6055b0b7d14c310000ae93352ed6754f70fa8f7c34141f91c4e3219", size = 30371, upload-time = "2025-11-21T23:01:54.787Z" } wheels = [ @@ -2517,7 +2517,7 @@ requires-dist = [ { name = "pydantic", marker = "extra == 'datasets'", specifier = ">=2" }, { name = "pydantic", marker = "extra == 'variables'", specifier = ">=2" }, { name = "pydantic-evals", marker = "extra == 'datasets'", specifier = ">=1.0.0" }, - { name = "pydantic-handlebars", marker = "extra == 'variables'", specifier = ">=0.2.0" }, + { name = "pydantic-handlebars", marker = "extra == 'variables'", specifier = ">=0.2.1" }, { name = "rich", specifier = ">=13.4.2" }, { name = "starlette", marker = "extra == 'gateway'", specifier = ">=0.37.0" }, { name = "tomli", marker = "python_full_version < '3.11'", specifier = ">=2.0.1" }, @@ -2595,7 +2595,7 @@ dev = [ { name = "pydantic", specifier = ">=2.11.0" }, { name = "pydantic-ai-slim", specifier = ">=0.0.39" }, { name = "pydantic-evals", specifier = ">=1.0.0" }, - { name = "pydantic-handlebars", specifier = ">=0.2.0" }, + { name = "pydantic-handlebars", specifier = ">=0.2.1" }, { name = "pymongo", specifier = ">=4.10.1" }, { name = "pymysql", specifier = ">=1.1.1" }, { name = "pyright", specifier = "!=1.1.407" }, @@ -4972,14 +4972,14 @@ wheels = [ [[package]] name = "pydantic-handlebars" -version = "0.2.0" +version = "0.2.1" source = { registry = "https://pypi.org/simple" } dependencies = [ { name = "pydantic" }, ] -sdist = { url = "https://files.pythonhosted.org/packages/2d/a3/13b1f17648605d1872bbc6cc56f24d9a2f4151bbf0623b9f731282a061be/pydantic_handlebars-0.2.0.tar.gz", hash = "sha256:11ee67abddefcb624ede8c690bc0210248ac235a150d9423908a89630c9a4e98", size = 175652, upload-time = "2026-05-22T06:06:38.476Z" } +sdist = { url = "https://files.pythonhosted.org/packages/96/73/e55a1fe1a8788a5fa82d9209e796f4111e28f2d2fecab7173aa6d80516ad/pydantic_handlebars-0.2.1.tar.gz", hash = "sha256:d4124cfbf7d6e3bded9331a08ccccf6f29f3e3a93665b35b5d6061650aeeb49f", size = 176949, upload-time = "2026-05-25T01:24:38.354Z" } wheels = [ - { url = "https://files.pythonhosted.org/packages/4d/f1/a27154170818efe3cb38af1eb54e0f7fc155873bd3b54f39a672a918e6cb/pydantic_handlebars-0.2.0-py3-none-any.whl", hash = "sha256:e5accc8ed0dc1bd953daa2eea2c0ee1eab7a6a27029da2439abacdf4ed46a4ae", size = 49954, upload-time = "2026-05-22T06:06:37.034Z" }, + { url = "https://files.pythonhosted.org/packages/55/11/364bc401f1d8fdb3947079fc43ffdfbfc9132d065981a03a95d2e87440c4/pydantic_handlebars-0.2.1-py3-none-any.whl", hash = "sha256:c713427d6498cf4b66814447d54753a2748f8a8d3a9f00c194192ddb3df61e52", size = 50476, upload-time = "2026-05-25T01:24:37.104Z" }, ] [[package]]