feat(template): support Jinja include/import/extends for prompts loaded via !file#291
Conversation
|
@microsoft-github-policy-service agree |
5841057 to
a09211d
Compare
jrob5756
left a comment
There was a problem hiding this comment.
Another great PR. Thanks @hertznsk ! I found two behavior issues in the file-backed rendering path. Both are worth fixing before merge because they can leave placeholders unresolved or turn a missing file into a misleading inline-template error.
| tmpl = self.env.from_string(template) | ||
| if isinstance(template, FileString) and template.source_path.exists(): | ||
| is_file_backed = True | ||
| loader = FileSystemLoader(str(template.source_path.parent)) |
There was a problem hiding this comment.
Jinja loads partials after the config loader has finished ${VAR} expansion. Variables in an included, imported, or base template therefore stay literal, while the same text in the main !file prompt is expanded. An unset required variable also stops producing the normal configuration error. Please run each template source through the existing environment resolver before Jinja compiles it, and add an end-to-end partial test.
There was a problem hiding this comment.
Fixed in b80e95e.
Each partial source now goes through the config loader's own env resolver before Jinja compiles it: _EnvResolvingFileSystemLoader(FileSystemLoader).get_source() runs resolve_env_vars(source) on the text of every {% include %} / {% import %} / {% extends %} target (nested and dynamic includes included), so ${VAR} / ${VAR:-default} in partials behaves identically to the root !file prompt. An unset required variable propagates as the original ConfigurationError (dedicated except ConfigurationError: raise ahead of the template-error handlers), preserving its message and suggestion.
Tests: unit coverage in tests/test_executor/test_template_filestring.py (render-time resolution, default fallback, unset-required ConfigurationError) plus an end-to-end test in tests/test_integration/test_file_prompt_includes.py that sets the env var after config load to prove resolution happens at render time.
| is_file_backed = False | ||
| try: | ||
| tmpl = self.env.from_string(template) | ||
| if isinstance(template, FileString) and template.source_path.exists(): |
There was a problem hiding this comment.
If the original file is removed or inaccessible after loading, this condition quietly treats the FileString as an inline prompt. Includes then fail with a message telling the user to switch to !file, even though they already did. Please report the unavailable source_path directly instead of changing rendering modes.
There was a problem hiding this comment.
Fixed in b80e95e.
The file-backed branch no longer degrades silently: TemplateRenderer.render() now checks template.source_path.is_file() for every FileString before entering the try block, and raises immediately:
TemplateError: File-backed prompt source is no longer available: '<path>' (loaded via !file).
with a suggestion to restore the file — no inline fallback, no misleading "convert to prompt: !file" guidance, and no double-wrapping by the generic handler (the tests assert the exact message and __cause__ is None). Note the check uses is_file(), so a directory or broken symlink at that path is reported the same way.
Tests: test_missing_source_path_raises_explicit_error (unit) and test_deleted_prompt_source_fails_with_explicit_error (end-to-end: load config, delete the prompt file, run the engine, assert the error names the missing path).
… prompt source
Jinja loads {% include %}/{% import %}/{% extends %} targets at render
time, after the config loader finished ${VAR} expansion — so partial
files rendered placeholders literally, and an unset required variable
silently passed through instead of raising the usual configuration
error. Partial sources now go through the same resolve_env_vars()
resolver before Jinja compiles them, and ConfigurationError propagates
unwrapped.
Also, a prompt file deleted after workflow load used to silently
downgrade the FileString to inline rendering, making includes fail
with a misleading "convert to prompt: !file" message. Rendering now
fails immediately with an explicit error naming the unavailable
source path.
Review feedback on PR microsoft#291 (issue microsoft#287).
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #291 +/- ##
=======================================
Coverage ? 89.33%
=======================================
Files ? 72
Lines ? 12778
Branches ? 0
=======================================
Hits ? 11415
Misses ? 1363
Partials ? 0 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Summary
Closes #287.
Prompt templates loaded via
prompt: !file ...(andsystem_prompt: !file ...) now support loader-dependent Jinja constructs —{% include %},{% import %}, and{% extends %}— resolved relative to the prompt file's own directory. This enables reusable prompt partials across workflows.The fix follows the suggested direction from the issue (options 1 + 2 + 3 + 4 combined): preserve the source path of
!file-loaded prompts, give the Jinja renderer aFileSystemLoaderrooted at that path, document the resolution rules, and produce clear error messages for both missing includes and inline prompts that attempt loader-dependent constructs.Changes
src/conductor/file_string.py(new leaf module):FileString(str)subclass carryingsource_path: Path;__getnewargs__keeps it safe acrosscopy.deepcopy/pickle/model_copy(deep=True).src/conductor/config/loader.py:!fileraw-string content is returned asFileString(dict/list parsing unchanged);_resolve_env_vars_recursive()preservesFileStringmetadata through${VAR}resolution.src/conductor/config/schema.py: Pydantic wrap-validators onAgentDef.prompt/system_promptpreserve theFileStringsubclass through validation; public annotations unchanged;model_dump(mode="json")still yields plain strings. Other!filefields (command,stdin,reason,value, ...) intentionally keep coercing to plainstr.src/conductor/executor/template.py:TemplateRenderer.render()branches onisinstance(FileString)— file-backed prompts render in a_DictSafeEnvironmentwithFileSystemLoader(source_path.parent)viaenv.compile(..., filename=...)+from_code(...); inline prompts use the unchangedBaseLoader+from_stringpath.TemplateNotFounderrors now name the searched directory (Template not found: '<name>'. Searched in: <dir>), and inline prompts with{% include %}get an explicit error explaining that loader-dependent constructs requireprompt: !file ....docs/workflow-syntax.md: new "Jinja Includes in Prompt Files" subsection with example, resolution rules, and the documented limitation thatconductor validatedoes not scan inside included files.prompt,system_prompt,for_eachinline agents, andhuman_gate(shared-renderer side effect), plus missing-include failure path.Design decisions (scope guardrails)
isinstance(FileString).human_gateprompts loaded via!filegain include support as a natural side effect of the shared renderer.Test plan
uv run pytest -m "not install_scripts" tests/— 3857 passed (1 pre-existing env-onlyreal_apifailure reproduced identically onmain, unrelated).make check(ruff lint + format + ty) — exit 0.uv run conductor validate(exit 0); verified rendered prompt/system_prompt reach the provider with partials inlined; verified both new error messages.