Add variable composition and Handlebars template rendering#1951
Add variable composition and Handlebars template rendering#1951dmontagu wants to merge 4 commits into
Conversation
c4b82ab to
f3de335
Compare
57a4253 to
4e23fad
Compare
Managed variables can now reference other variables and render
Handlebars templates against typed inputs.
Composition: `@{variable_name}@` references in serialized variable
values are expanded during resolution, before deserialization. The
resolver walks a small Handlebars-compatible subset of `@{}@` block
helpers (top-level `#if`/`#each`) and supports dotted-path access
(`@{user.name}@`). Resolution priority is shared between top-level
variables and child references via `_BaseVariable._lookup_serialized`:
context override → provider → registered code default. Reference
cycles, missing references, and depth limits surface through
`ComposedReference.error` and propagate as a single composition
warning at resolution time, with the resolver falling back to the
code default. Each resolved variable also carries a `composed_from`
trail (with reason/label/version per ref) onto the span attributes.
Template rendering: a new `logfire.template_var()` registers a
`TemplateVariable[T, InputsT]` whose `.get(inputs)` resolves the
variable, expands `@{ref}@` references, renders any `{{placeholder}}`
expressions using `pydantic_handlebars`, and deserialises to the
declared type. `inputs_type` generates a `template_inputs_schema`
that is included in the variable config and synced to the server.
`pydantic-handlebars` is an optional dependency installed via the
`logfire[variables]` extra on Python 3.10+; calling `template_var()`
without it raises immediately rather than silently degrading.
`Variable` is now a thin subclass of an internal `_BaseVariable`
that holds the shared resolution pipeline. The base class carries
no template-related surface area; `TemplateVariable` overrides
`to_config` to attach `template_inputs_schema`, and external diff/
sync code gates on `isinstance(variable, TemplateVariable)` via the
`get_template_inputs_schema(variable)` helper.
Write-time validation lives in `logfire.variables.template_validation`
and uses `pydantic_handlebars.check_template_compatibility` to detect
undeclared `{{field}}` references across the composition graph. Cycle
detection on the reference graph is also exposed for push-time use.
- Adds `docs/reference/advanced/managed-variables/templates-and-composition.md`
covering Handlebars `{{placeholder}}` rendering via `logfire.template_var()`
and `@{variable_name}@` composition references, including structured
values, cycle detection, and how templates and composition combine.
- Expands the managed-variables index with a templates intro and links
to the new page.
- Mentions `template_inputs_schema` and the `[variables]` extra in the
configuration reference and nav.
- Adds `examples/python/variable_composition_demo.py` exercising
composition, structured variables, template inputs, and
composition-time conditionals end-to-end.
- Skips Python doc examples whose source mentions `logfire.template_var`
when `pydantic-handlebars` is unavailable (matches the runtime
requirement on Python 3.9).
4e23fad to
e80febf
Compare
| if context_overrides is not None and name in context_overrides and variable is not None: | ||
| override_value = context_overrides[name] | ||
| if is_resolve_function(override_value): | ||
| override_value = override_value(targeting_key, attributes) | ||
| try: | ||
| serialized = variable.type_adapter.dump_json(override_value).decode('utf-8') | ||
| except (ValueError, TypeError, RuntimeError): | ||
| pass # Fall through to provider/code default | ||
| else: | ||
| return ResolvedVariable(name=name, value=serialized, reason='context_override') |
There was a problem hiding this comment.
🚩 Behavioral change: context overrides now serialize/deserialize round-trip
The old _resolve returned context overrides as typed Python objects directly (return ResolvedVariable(name=self.name, value=context_value, reason='context_override')). The new code in _lookup_serialized (variable.py:284-293) serializes the override via dump_json, and _resolve (variable.py:227-232) then deserializes it back via validate_json. This round-trip can silently drop overrides that fail serialization (the except at line 290 falls through to the provider/code default). While this is tested (test_unserializable_override_falls_through_to_provider), it's a semantic change from the old behavior where overrides always took effect regardless of serializability. The change is motivated by the need to support composition and template rendering on override values, but callers who relied on overrides with non-serializable types will see silent fallthrough.
Was this helpful? React with 👍 or 👎 to provide feedback.
| try: | ||
| serialized = variable.type_adapter.dump_json(override_value).decode('utf-8') | ||
| except (ValueError, TypeError, RuntimeError): | ||
| pass # Fall through to provider/code default |
There was a problem hiding this comment.
This silent fallthrough seems user-unfriendly for an explicit override(...). If an override value cannot be serialized for composition/template processing, we should probably emit a warning at minimum before ignoring it and using provider/code-default resolution. Otherwise tests can pass while callers think their override took effect when it did not.
There was a problem hiding this comment.
also, if unserializable overrides aren't allowed, why are unserializable code defaults allowed? this is weird:
import logfire
x1 = object()
x2 = object()
fragment = logfire.var(name='fragment', default=x1, type=object)
with fragment.override(x2):
assert fragment.get().value is x1| self, | ||
| name: str, | ||
| *, | ||
| type: type[T], |
There was a problem hiding this comment.
Question: var() lets type be omitted and infers it from a non-callable default, while template_var() requires type even though it also has a non-callable default in the common case. Is that difference intentional? If not, it seems like template_var("prompt", default="Hello {{name}}", inputs_type=Inputs) could mirror var() and only require type when the default is a resolve function.
| ref_graph: dict[str, set[str]] = {} | ||
|
|
||
| # Scan local variable defaults for references | ||
| for variable in variables: |
There was a problem hiding this comment.
P2: This only builds the reference graph from variables passed in by local code, so refs inside server-only variables reached through composition are not validated.
Concrete remote repro from scratch_2934.py:
logfire.configure()
foo2 = logfire.var('foo2', type=str, default='foo2 references @{foo1}@')
print(logfire.variables_validate())
foo1 = logfire.var('foo1', type=str, default='foo1')
print(logfire.variables_validate())With remote foo1.latest_version = "foo1 references @{foo3}@", the first validation reports reference_errors=[] because local foo2 references foo1 and foo1 exists remotely. Only after registering local foo1 does validation scan foo1's remote values and report Variable 'foo1' references '@{foo3}@' which does not exist.
So strict validation can miss missing refs/cycles inside server-only composed fragments that are reachable from local variables. It probably needs to traverse the composed reference graph, not just scan server values for names that are already local variables.
There was a problem hiding this comment.
The backend just let me set that invalid template foo1 references @{foo3}@, i'm not sure if that's an issue on its own.
Also I think validating foo2 should complain about the missing local var even when the server remote value is valid. Fallback to the code default should always be an option.
| composed: list[ComposedReference] = [] | ||
|
|
||
| # Expand @{references}@ if any are present | ||
| if has_references(serialized_value): |
There was a problem hiding this comment.
This guard makes escaped-only composition syntax behave differently from the same escaped syntax when a real reference is also present. Repro on this stack:
import logfire
logfire.configure()
baz = logfire.var('baz', type=str, default='baz')
assert logfire.var('bar1', type=str, default=r'@{baz}@').get().value == 'baz'
assert logfire.var('bar2', type=str, default=r'\@{baz}@').get().value == r'\@{baz}@'
assert logfire.var('bar3', type=str, default=r'@{baz}@ and \@{baz}@').get().value == 'baz and @{baz}@'The escaped @{baz}@ keeps the backslash when it is the only composition-looking syntax, because has_references(serialized_value) is false and expand_references() never gets a chance to unescape it. When there is also a real reference, expansion runs and the escaped reference becomes a literal @{baz}@. So the observable escaping behavior depends on whether another reference is present in the same value.
| ) -> tuple[str, list[ComposedReference]]: | ||
| """Expand `@{var}@` references in a serialized variable value. | ||
|
|
||
| Uses the Handlebars engine so that `@{}@` supports simple references, |
There was a problem hiding this comment.
This docstring also understates the syntax after #1952. The runtime is now using native pydantic-handlebars dependency extraction/rendering, so block helper conditions are not limited to top-level names; for example @{#if user.active}@...@{/if}@ works. Keeping this wording makes the code comments disagree with the behavior introduced later in the stack.
| | `@{#if variable}@...@{else}@...@{/if}@` | Conditional on whether a variable is set | | ||
| | `@{#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}@`. |
There was a problem hiding this comment.
This restriction becomes stale once #1952 is applied. The native Handlebars path accepts dotted expressions in block helper headers, e.g. @{#if user.active}@premium@{/if}@ renders correctly (I verified it returns premium). So the docs should either remove the top-level-only restriction here or explicitly document the broader syntax now supported by #1952.
| # 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')) |
There was a problem hiding this comment.
This looks worth tightening. The templated value itself is not really "bad" here: Config.model_construct(code="{{code}}") is valid as an unrendered template, and the rendered result only becomes invalid because Inputs(code="abc123") violates the ^[A-Z]+$ constraint. Could this use names like templated_config and invalid_inputs to make that clearer? Also, if this behavior is intentional, should the fallback path emit a warning when render inputs produce a value that fails validation before returning the code default with reason="other_error"?
Six items Alex flagged as important-but-not-blocking on the original 1951/1952 stack, folded in before #1954 merges: ## Docs - `composition.py` module docstring + `expand_references` docstring no longer claim block helpers are restricted to top-level identifiers — the native handlebars path accepts `@{#if user.active}@` and helper sub-expression headers. - `docs/.../templates-and-composition.md` Control Flow section rewritten to match: drops the "must be top-level" caveat and adds rows for dotted-path conditions and `../` parent-scope access from inside a block. ## Caching - New `compile_composition_template(source)` in `_handlebars` wraps the parse step in an `lru_cache(maxsize=1024)` (#1952 r3289095058). `reference_syntax.render_once` now compiles once per distinct source and reuses the compiled program across resolutions — managed-variable values are typically stable, so the hit rate should be high. - `extract_composition_dependencies` no longer re-runs the `try: from pydantic_handlebars import …` block on every call — the import is moved into a `@cache`d helper that returns the function object directly. Matches the pattern of the existing `_get_template_compatibility_checker` (#1952 r3288194502). ## Warning text - `_composition_failure` renamed to `_fallback_to_default` and takes a `failure_stage` argument (`'composition'` or `'template rendering'`) so the `RuntimeWarning` text reflects the actual failed step. A pydantic-handlebars parse error during `{{...}}` rendering used to surface as `"composition failed"` even though composition succeeded (codex finding). - Updated `test_remote_render_error_records_exception` accordingly. ## Test naming - `test_override_render_failure_falls_back` renamed its locals: `bad_override` → `templated_config` (it's a *valid* template), and introduced `invalid_inputs` for the inputs that actually fail the pattern constraint. Alex r3289312130. ## Escape-detection coupling - Added a comment on the `_HAS_REFERENCE` regex flagging that the lookbehind encodes pydantic-handlebars' current escape semantics ("any preceding `\` escapes") and noting it'll need to count preceding backslashes if pydantic-handlebars adopts Handlebars.js's odd-vs-even-backslash spec behaviour (#1952 r3289062247).
Two places were silent on how `Variable.override(...)` interacts with composition and template rendering — important behaviour to spell out, not least because the pre-#1962 silent-fallthrough bug was visible to users without any docs to refer to. ## `override()` docstring (`variable.py`) Expanded the one-sentence summary into three explicit sections: - **Composition is skipped** — overriding with `'hi @{user}@'` yields the literal string; `@{user}@` is not expanded. Reasoning: overrides are the user's literal choice, not a template for the composition graph to resolve. - **Template rendering still applies to `TemplateVariable`** — as long as the override value is JSON-serializable, `{{...}}` rendering against the runtime `inputs` happens the same way as for a provider value. - **Unserializable overrides come back verbatim** — no serialize/deserialize round-trip, no render pass; matches the "literal user choice" intent (and is the pre-#1951 behaviour restored). ## User docs (`configuration-reference.md`) Added a `## Overrides and composition` subsection under "Contextual Overrides" with the same two-bullet summary and a runnable snippet demonstrating both states — composition expanding `@{user}@` from the default vs. override returning `'Hi @{user}@'` literally. Runs under `pytest tests/test_docs.py`.
|
Closing in favor of #1954 |
Re-opens the work from #1731 with a cleaner two-commit history (one for code, one for docs/demo) so this PR is easier to browse and review.
The diff has been polished since #1731:
TemplateVariablenow subclassesVariabledirectly (rather than both subclassing an internal_BaseVariable), so the public API surface no longer exposes a private base class. The.get(inputs, …)override onTemplateVariableis a knowing Liskov violation marked with# pyright: ignore[reportIncompatibleMethodOverride].tests/test_docs.pynow skips bothlogfire.template_varand@{…}@examples on Python 3.9 (composition requirespydantic-handlebars, which the[variables]extra installs only on 3.10+).Everything else is functionally identical to
feature/variable-composition.Summary
@{variable_name}@syntax. Composition happens during resolution, before deserialization, and supports nested references, dotted access like@{brand.tagline}@, escaping, cycle/depth protection, andcomposed_frommetadata on resolved variables and spans.{{placeholder}}templates rendered with runtime inputs. Rendering usespydantic-handlebarsand preserves{{...}}placeholders while@{...}@composition is expanded first.TemplateVariable[T, InputsT]API:logfire.template_var()adds a single-step flow whereget(inputs)resolves the variable, expands@{...}@references, renders{{...}}placeholders, and deserializes to the declared type.TemplateVariableis a subclass ofVariable.Variable._lookup_serializedencodes the override → provider → registered code default priority once, and routes both_resolve(forself.name) and the composition expander'sresolve_ref(for child@{ref}@lookups) through it.Follow-ups (not blocking this PR)
@{}@syntax parity with{{}}(block helpers, dotted paths, etc.).Supersedes
This PR replaces #1731. All review threads on #1731 have been addressed and resolved. #1731 will be closed once this PR is reviewed and merged.