Skip to content

Commit bcf63a2

Browse files
fix: CoPilot review round-2 pass on PR #45
- CHANGELOG.md: update 12 → 16 hex chars to match the widened FilesystemPromptBackend.version derivation. - prompt.py: PromptResult.messages gains Field(min_length=1) so the spec §4 'Ordered non-empty sequence' mandate is enforced at the type boundary, not just by the construction path. - errors.py: PromptStoreUnavailable gains an optional causes list[BaseException] attribute carrying per-backend exceptions index-aligned to backends_tried. - manager.py: aggregate raise populates causes with the per-backend exceptions in fallback order, while keeping the __cause__ chain pointing at the last unavailable for stack-trace continuity. - manager.py: PromptManager carries a per-instance dict[str, jinja2.Template] keyed by template_hash. Render consults the cache and only re-parses on miss. Unbounded for v1 (typical apps have O(10) prompts; an LRU follow-on can land if benchmarks show memory pressure). template_hash is content-derived, so cache invalidation is automatic when a backend returns updated content. - test_prompts.py: new tests for empty-messages rejection and for the compiled-template cache hit behavior.
1 parent 85b3561 commit bcf63a2

5 files changed

Lines changed: 59 additions & 10 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). The
88

99
### Added
1010

11-
- **Prompt-management capability (proposal 0017, introduced in spec v0.15.0).** New `openarmature.prompts` subpackage. `PromptManager` composes one or more `PromptBackend`s, exposes `fetch` / `render` / `get`, applies the §8 fallback semantics (`prompt_store_unavailable` continues to the next backend; `prompt_not_found` stops the chain), and renders templates with Jinja2's `StrictUndefined` per §7. `Prompt` / `PromptResult` / `PromptGroup` are Pydantic models matching spec §3 / §4 / §9. Three error categories (`PromptNotFound`, `PromptRenderError`, `PromptStoreUnavailable`) with `PROMPT_TRANSIENT_CATEGORIES` exported for retry-middleware classifiers. `FilesystemPromptBackend` is the minimum local-filesystem reference backend (layout: `<root>/<label>/<name>.j2`; `version` derived from the first 12 chars of `template_hash`). New runtime dependency: `jinja2>=3.1`.
11+
- **Prompt-management capability (proposal 0017, introduced in spec v0.15.0).** New `openarmature.prompts` subpackage. `PromptManager` composes one or more `PromptBackend`s, exposes `fetch` / `render` / `get`, applies the §8 fallback semantics (`prompt_store_unavailable` continues to the next backend; `prompt_not_found` stops the chain), and renders templates with Jinja2's `StrictUndefined` per §7. `Prompt` / `PromptResult` / `PromptGroup` are Pydantic models matching spec §3 / §4 / §9. Three error categories (`PromptNotFound`, `PromptRenderError`, `PromptStoreUnavailable`) with `PROMPT_TRANSIENT_CATEGORIES` exported for retry-middleware classifiers. `FilesystemPromptBackend` is the minimum local-filesystem reference backend (layout: `<root>/<label>/<name>.j2`; `version` derived from the first 16 hex chars of `template_hash`). New runtime dependency: `jinja2>=3.1`.
1212
- **`openarmature.prompts.context` — observability propagation per spec §11.** `with_active_prompt(result)` and `with_active_prompt_group(group)` context managers + `current_prompt_result()` / `current_prompt_group()` inspectors. When the OTel observer is active and an LLM call fires inside `with_active_prompt`, the `openarmature.llm.complete` span carries the normative `openarmature.prompt.*` attributes (`name`, `version`, `label`, `template_hash`, `rendered_hash`, `group_name`). Nesting is innermost-wins.
1313
- **Image content blocks for user messages (proposal 0015, introduced in spec v0.13.0).** `UserMessage.content` now accepts `str | list[ContentBlock]`. The block surface introduces `TextBlock`, `ImageBlock`, `ImageSourceURL`, `ImageSourceInline`, and the `ContentBlock` / `ImageSource` discriminated unions over the block / source `type` field. `ImageBlock` carries a `media_type` (required for inline sources; ignored for URL sources; typed as `str | None` so callers MAY pass any `image/*` type the bound model supports) and an optional `detail` hint (`"auto"` / `"low"` / `"high"`; `None` default omits the field from the wire so providers apply their own default). System, assistant, and tool messages stay text-string-only; image inputs are user-only in v1.
1414
- **`OpenAIProvider` content-array wire mapping.** When `UserMessage.content` is a content-block sequence, the wire body uses OpenAI's `content` array per §8.1.1. `TextBlock → {type: "text", text}`. `ImageBlock` with a URL source maps to `{type: "image_url", image_url: {url, detail?}}`. `ImageBlock` with an inline source constructs an RFC 2397 `data:<media_type>;base64,<base64_data>` URI and goes through the same `image_url` entry shape. Inline bytes pass through unchanged — no inspection, transcoding, or re-encoding.

src/openarmature/prompts/errors.py

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -96,24 +96,30 @@ class PromptStoreUnavailable(PromptError):
9696
Transient: the same fetch may succeed when the backend recovers.
9797
``PromptManager.fetch`` raises this only after ALL composed
9898
backends raise it; in that aggregate case ``backends_tried``
99-
lists the backends consulted (in order) for operator visibility.
100-
The ``__cause__`` chain preserves per-backend failure reasons.
99+
lists the backends consulted (in order) and ``causes`` carries
100+
the per-backend exceptions index-aligned to ``backends_tried``
101+
so operators can distinguish "backend A 503 + backend B 503"
102+
from "backend A 503 + backend B OSError". The ``__cause__`` chain
103+
still points at the last unavailable for stack-trace continuity.
101104
"""
102105

103106
category = PROMPT_STORE_UNAVAILABLE
104107

105108
name: str | None
106109
label: str | None
107110
backends_tried: list[str] | None
111+
causes: list[BaseException] | None
108112

109113
def __init__(
110114
self,
111115
*args: Any,
112116
name: str | None = None,
113117
label: str | None = None,
114118
backends_tried: list[str] | None = None,
119+
causes: list[BaseException] | None = None,
115120
) -> None:
116121
super().__init__(*args)
117122
self.name = name
118123
self.label = label
119124
self.backends_tried = backends_tried
125+
self.causes = causes

src/openarmature/prompts/manager.py

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,13 @@ def __init__(self, *backends: PromptBackend) -> None:
4747
if not backends:
4848
raise ValueError("PromptManager requires at least one backend")
4949
self._backends: tuple[PromptBackend, ...] = backends
50+
# template_hash → compiled jinja2 Template. Per-manager,
51+
# unbounded. Correct by construction: template_hash is
52+
# content-derived, so a backend returning updated content
53+
# surfaces a fresh hash and a fresh cache entry. An LRU
54+
# eviction policy can land if benchmarks ever show memory
55+
# pressure; typical apps have O(10) prompts.
56+
self._template_cache: dict[str, jinja2.Template] = {}
5057

5158
async def fetch(self, name: str, label: str = "production") -> Prompt:
5259
"""Consult composed backends in order, applying §8 fallback.
@@ -59,28 +66,29 @@ async def fetch(self, name: str, label: str = "production") -> Prompt:
5966
next. After ALL backends are exhausted with unavailable
6067
failures, the manager raises ``PromptStoreUnavailable``.
6168
"""
62-
last_unavailable: PromptStoreUnavailable | None = None
69+
causes: list[BaseException] = []
6370
for backend in self._backends:
6471
try:
6572
return await backend.fetch(name, label)
6673
except PromptNotFound:
6774
raise
6875
except PromptStoreUnavailable as exc:
69-
last_unavailable = exc
76+
causes.append(exc)
7077
_log.warning(
7178
"prompt backend %r unavailable for (%r, %r); falling back",
7279
backend,
7380
name,
7481
label,
7582
)
7683
continue
77-
assert last_unavailable is not None
84+
assert causes
7885
raise PromptStoreUnavailable(
7986
f"all prompt backends unavailable for ({name!r}, {label!r})",
8087
name=name,
8188
label=label,
8289
backends_tried=[type(b).__name__ for b in self._backends],
83-
) from last_unavailable
90+
causes=list(causes),
91+
) from causes[-1]
8492

8593
def render(
8694
self,
@@ -103,7 +111,10 @@ def render(
103111

104112
rendered_text: str
105113
try:
106-
template = _RENDER_ENV.from_string(prompt.template)
114+
template = self._template_cache.get(prompt.template_hash)
115+
if template is None:
116+
template = _RENDER_ENV.from_string(prompt.template)
117+
self._template_cache[prompt.template_hash] = template
107118
rendered_text = template.render(**variables)
108119
except jinja2.UndefinedError as exc:
109120
raise PromptRenderError(

src/openarmature/prompts/prompt.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
from datetime import datetime
66
from typing import Any
77

8-
from pydantic import BaseModel, ConfigDict
8+
from pydantic import BaseModel, ConfigDict, Field
99

1010
from openarmature.llm.messages import Message
1111

@@ -83,7 +83,7 @@ class PromptResult(BaseModel):
8383
label: str
8484
template_hash: str
8585
rendered_hash: str
86-
messages: list[Message]
86+
messages: list[Message] = Field(min_length=1)
8787
variables: dict[str, Any]
8888
fetched_at: datetime
8989
rendered_at: datetime

tests/unit/test_prompts.py

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,22 @@ def test_prompt_extra_fields_forbidden() -> None:
149149
)
150150

151151

152+
def test_prompt_result_rejects_empty_messages() -> None:
153+
prompt = _make_prompt()
154+
with pytest.raises(ValueError):
155+
PromptResult(
156+
name=prompt.name,
157+
version=prompt.version,
158+
label=prompt.label,
159+
template_hash=prompt.template_hash,
160+
rendered_hash="sha256:abc",
161+
messages=[],
162+
variables={},
163+
fetched_at=prompt.fetched_at,
164+
rendered_at=datetime.now(UTC),
165+
)
166+
167+
152168
def test_prompt_group_rejects_zero_members() -> None:
153169
with pytest.raises(ValueError, match="at least two"):
154170
PromptGroup(group_name="g", members=[])
@@ -375,6 +391,22 @@ async def fetch(self, name: str, label: str = "production") -> Prompt:
375391
assert second.calls == 0
376392

377393

394+
def test_manager_render_caches_compiled_templates_by_hash() -> None:
395+
prompt = _make_prompt()
396+
397+
class _Backend:
398+
async def fetch(self, name: str, label: str = "production") -> Prompt:
399+
return prompt
400+
401+
manager = PromptManager(_Backend())
402+
manager.render(prompt, {"user": "Alice"})
403+
assert prompt.template_hash in manager._template_cache # noqa: SLF001
404+
cached = manager._template_cache[prompt.template_hash] # noqa: SLF001
405+
# Second render reuses the same compiled Template instance.
406+
manager.render(prompt, {"user": "Bob"})
407+
assert manager._template_cache[prompt.template_hash] is cached # noqa: SLF001
408+
409+
378410
async def test_manager_render_signature_returns_user_message() -> None:
379411
prompt = _make_prompt()
380412

0 commit comments

Comments
 (0)