fix(prompts): fix three infrastructure bugs in prompt dependency resolution
- ContentMacro: add __post_init__ to auto-derive raw_text when built
programmatically, preventing str.replace("", X) corruption
- MacroParser: add @{target} shorthand syntax support mapped to REQUIRED kind,
updating parse, has_macros, count_macros, and find_macro_positions
- Artifact: store content in model and SQLite DB, replace resolver placeholder
with actual artifact content, add migration for existing databases
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -89,6 +89,7 @@ class Artifact:
|
|||||||
artifact_type: ArtifactType
|
artifact_type: ArtifactType
|
||||||
content_digest: str
|
content_digest: str
|
||||||
content_size: int = 0
|
content_size: int = 0
|
||||||
|
content: str = ""
|
||||||
metadata: ArtifactMetadata = field(default_factory=ArtifactMetadata)
|
metadata: ArtifactMetadata = field(default_factory=ArtifactMetadata)
|
||||||
created_at: datetime = field(default_factory=datetime.utcnow)
|
created_at: datetime = field(default_factory=datetime.utcnow)
|
||||||
updated_at: datetime = field(default_factory=datetime.utcnow)
|
updated_at: datetime = field(default_factory=datetime.utcnow)
|
||||||
@@ -126,6 +127,7 @@ class Artifact:
|
|||||||
artifact_type=artifact_type,
|
artifact_type=artifact_type,
|
||||||
content_digest=content_digest,
|
content_digest=content_digest,
|
||||||
content_size=content_size,
|
content_size=content_size,
|
||||||
|
content=content,
|
||||||
metadata=metadata or ArtifactMetadata(),
|
metadata=metadata or ArtifactMetadata(),
|
||||||
)
|
)
|
||||||
|
|
||||||
@@ -136,6 +138,7 @@ class Artifact:
|
|||||||
Args:
|
Args:
|
||||||
new_content: New content string
|
new_content: New content string
|
||||||
"""
|
"""
|
||||||
|
self.content = new_content
|
||||||
self.content_digest = calculate_content_digest(new_content)
|
self.content_digest = calculate_content_digest(new_content)
|
||||||
self.content_size = len(new_content.encode('utf-8'))
|
self.content_size = len(new_content.encode('utf-8'))
|
||||||
self.updated_at = datetime.utcnow()
|
self.updated_at = datetime.utcnow()
|
||||||
@@ -161,6 +164,7 @@ class Artifact:
|
|||||||
"artifact_type": self.artifact_type.value,
|
"artifact_type": self.artifact_type.value,
|
||||||
"content_digest": self.content_digest,
|
"content_digest": self.content_digest,
|
||||||
"content_size": self.content_size,
|
"content_size": self.content_size,
|
||||||
|
"content": self.content,
|
||||||
"metadata": self.metadata.to_dict(),
|
"metadata": self.metadata.to_dict(),
|
||||||
"created_at": self.created_at.isoformat(),
|
"created_at": self.created_at.isoformat(),
|
||||||
"updated_at": self.updated_at.isoformat(),
|
"updated_at": self.updated_at.isoformat(),
|
||||||
@@ -176,6 +180,7 @@ class Artifact:
|
|||||||
artifact_type=ArtifactType(data["artifact_type"]),
|
artifact_type=ArtifactType(data["artifact_type"]),
|
||||||
content_digest=data["content_digest"],
|
content_digest=data["content_digest"],
|
||||||
content_size=data.get("content_size", 0),
|
content_size=data.get("content_size", 0),
|
||||||
|
content=data.get("content", ""),
|
||||||
metadata=ArtifactMetadata.from_dict(data.get("metadata", {})),
|
metadata=ArtifactMetadata.from_dict(data.get("metadata", {})),
|
||||||
created_at=datetime.fromisoformat(data["created_at"]),
|
created_at=datetime.fromisoformat(data["created_at"]),
|
||||||
updated_at=datetime.fromisoformat(data["updated_at"]),
|
updated_at=datetime.fromisoformat(data["updated_at"]),
|
||||||
|
|||||||
@@ -30,6 +30,7 @@ CREATE TABLE IF NOT EXISTS prompt_artifacts (
|
|||||||
artifact_type TEXT NOT NULL,
|
artifact_type TEXT NOT NULL,
|
||||||
content_digest TEXT NOT NULL,
|
content_digest TEXT NOT NULL,
|
||||||
content_size INTEGER DEFAULT 0,
|
content_size INTEGER DEFAULT 0,
|
||||||
|
content TEXT DEFAULT '',
|
||||||
metadata JSON,
|
metadata JSON,
|
||||||
created_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP,
|
created_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP,
|
||||||
updated_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP,
|
updated_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP,
|
||||||
@@ -59,6 +60,13 @@ def initialize_artifact_tables(db_path: str) -> None:
|
|||||||
conn = sqlite3.connect(db_path)
|
conn = sqlite3.connect(db_path)
|
||||||
try:
|
try:
|
||||||
conn.executescript(ARTIFACT_TABLES_SQL)
|
conn.executescript(ARTIFACT_TABLES_SQL)
|
||||||
|
# Migration: add content column to existing databases
|
||||||
|
try:
|
||||||
|
conn.execute(
|
||||||
|
"ALTER TABLE prompt_artifacts ADD COLUMN content TEXT DEFAULT ''"
|
||||||
|
)
|
||||||
|
except sqlite3.OperationalError:
|
||||||
|
pass # Column already exists
|
||||||
conn.commit()
|
conn.commit()
|
||||||
finally:
|
finally:
|
||||||
conn.close()
|
conn.close()
|
||||||
@@ -107,8 +115,8 @@ class SQLiteArtifactRepository(IArtifactRepository):
|
|||||||
"""
|
"""
|
||||||
INSERT INTO prompt_artifacts (
|
INSERT INTO prompt_artifacts (
|
||||||
id, space_id, name, artifact_type, content_digest,
|
id, space_id, name, artifact_type, content_digest,
|
||||||
content_size, metadata, created_at, updated_at
|
content_size, content, metadata, created_at, updated_at
|
||||||
) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?)
|
) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?)
|
||||||
""",
|
""",
|
||||||
(
|
(
|
||||||
artifact.id,
|
artifact.id,
|
||||||
@@ -117,6 +125,7 @@ class SQLiteArtifactRepository(IArtifactRepository):
|
|||||||
artifact.artifact_type.value,
|
artifact.artifact_type.value,
|
||||||
artifact.content_digest,
|
artifact.content_digest,
|
||||||
artifact.content_size,
|
artifact.content_size,
|
||||||
|
artifact.content,
|
||||||
json.dumps(artifact.metadata.to_dict()),
|
json.dumps(artifact.metadata.to_dict()),
|
||||||
artifact.created_at.isoformat(),
|
artifact.created_at.isoformat(),
|
||||||
artifact.updated_at.isoformat(),
|
artifact.updated_at.isoformat(),
|
||||||
@@ -251,12 +260,14 @@ class SQLiteArtifactRepository(IArtifactRepository):
|
|||||||
cursor = conn.execute(
|
cursor = conn.execute(
|
||||||
"""
|
"""
|
||||||
UPDATE prompt_artifacts
|
UPDATE prompt_artifacts
|
||||||
SET content_digest = ?, content_size = ?, metadata = ?, updated_at = ?
|
SET content_digest = ?, content_size = ?, content = ?,
|
||||||
|
metadata = ?, updated_at = ?
|
||||||
WHERE id = ?
|
WHERE id = ?
|
||||||
""",
|
""",
|
||||||
(
|
(
|
||||||
artifact.content_digest,
|
artifact.content_digest,
|
||||||
artifact.content_size,
|
artifact.content_size,
|
||||||
|
artifact.content,
|
||||||
json.dumps(artifact.metadata.to_dict()),
|
json.dumps(artifact.metadata.to_dict()),
|
||||||
artifact.updated_at.isoformat(),
|
artifact.updated_at.isoformat(),
|
||||||
artifact.id,
|
artifact.id,
|
||||||
@@ -326,6 +337,7 @@ class SQLiteArtifactRepository(IArtifactRepository):
|
|||||||
artifact_type=ArtifactType(row["artifact_type"]),
|
artifact_type=ArtifactType(row["artifact_type"]),
|
||||||
content_digest=row["content_digest"],
|
content_digest=row["content_digest"],
|
||||||
content_size=row["content_size"],
|
content_size=row["content_size"],
|
||||||
|
content=row["content"] or "",
|
||||||
metadata=ArtifactMetadata.from_dict(metadata_dict),
|
metadata=ArtifactMetadata.from_dict(metadata_dict),
|
||||||
created_at=datetime.fromisoformat(row["created_at"]),
|
created_at=datetime.fromisoformat(row["created_at"]),
|
||||||
updated_at=datetime.fromisoformat(row["updated_at"]),
|
updated_at=datetime.fromisoformat(row["updated_at"]),
|
||||||
|
|||||||
@@ -143,9 +143,7 @@ class PromptResolver:
|
|||||||
)
|
)
|
||||||
|
|
||||||
if artifact:
|
if artifact:
|
||||||
# Found! Get content (would need to load from storage in real impl)
|
content = artifact.content
|
||||||
# For now, we'll use a placeholder
|
|
||||||
content = f"[Content of {artifact.name} from {space_id}]"
|
|
||||||
|
|
||||||
resolved = ResolvedMacro(
|
resolved = ResolvedMacro(
|
||||||
macro=macro,
|
macro=macro,
|
||||||
|
|||||||
@@ -46,6 +46,11 @@ class ContentMacro:
|
|||||||
raw_text: str = ""
|
raw_text: str = ""
|
||||||
line_number: int = 0
|
line_number: int = 0
|
||||||
|
|
||||||
|
def __post_init__(self) -> None:
|
||||||
|
"""Auto-derive raw_text when built programmatically."""
|
||||||
|
if not self.raw_text:
|
||||||
|
self.raw_text = f"@{{{self.target}}}"
|
||||||
|
|
||||||
def __str__(self) -> str:
|
def __str__(self) -> str:
|
||||||
"""String representation of macro."""
|
"""String representation of macro."""
|
||||||
params = ''.join(f"|{k}={v}" for k, v in self.parameters.items())
|
params = ''.join(f"|{k}={v}" for k, v in self.parameters.items())
|
||||||
|
|||||||
@@ -38,6 +38,9 @@ class MacroParser:
|
|||||||
re.IGNORECASE
|
re.IGNORECASE
|
||||||
)
|
)
|
||||||
|
|
||||||
|
# Shorthand pattern: @{target} — maps to MacroKind.REQUIRED
|
||||||
|
SHORTHAND_PATTERN = re.compile(r'@\{([^}]+)\}')
|
||||||
|
|
||||||
# Parameter pattern: |key=value
|
# Parameter pattern: |key=value
|
||||||
PARAM_PATTERN = re.compile(r'\|([^=]+)=([^|]+)')
|
PARAM_PATTERN = re.compile(r'\|([^=]+)=([^|]+)')
|
||||||
|
|
||||||
@@ -95,6 +98,19 @@ class MacroParser:
|
|||||||
f"Line {line_number}: {e}"
|
f"Line {line_number}: {e}"
|
||||||
) from e
|
) from e
|
||||||
|
|
||||||
|
# Scan for @{target} shorthand syntax
|
||||||
|
for match in self.SHORTHAND_PATTERN.finditer(line):
|
||||||
|
target = match.group(1).strip()
|
||||||
|
raw_text = match.group(0)
|
||||||
|
if target:
|
||||||
|
macros.append(ContentMacro(
|
||||||
|
kind=MacroKind.REQUIRED,
|
||||||
|
target=target,
|
||||||
|
parameters={},
|
||||||
|
raw_text=raw_text,
|
||||||
|
line_number=line_number,
|
||||||
|
))
|
||||||
|
|
||||||
return macros
|
return macros
|
||||||
|
|
||||||
def _parse_match(self, match: re.Match, line_number: int) -> ContentMacro:
|
def _parse_match(self, match: re.Match, line_number: int) -> ContentMacro:
|
||||||
@@ -172,7 +188,7 @@ class MacroParser:
|
|||||||
content: Template content
|
content: Template content
|
||||||
|
|
||||||
Returns:
|
Returns:
|
||||||
List of (start_pos, end_pos, macro_text) tuples
|
List of (start_pos, end_pos, macro_text) tuples sorted by position
|
||||||
"""
|
"""
|
||||||
positions = []
|
positions = []
|
||||||
for match in self.MACRO_PATTERN.finditer(content):
|
for match in self.MACRO_PATTERN.finditer(content):
|
||||||
@@ -181,6 +197,13 @@ class MacroParser:
|
|||||||
match.end(),
|
match.end(),
|
||||||
match.group(0)
|
match.group(0)
|
||||||
))
|
))
|
||||||
|
for match in self.SHORTHAND_PATTERN.finditer(content):
|
||||||
|
positions.append((
|
||||||
|
match.start(),
|
||||||
|
match.end(),
|
||||||
|
match.group(0)
|
||||||
|
))
|
||||||
|
positions.sort(key=lambda p: p[0])
|
||||||
return positions
|
return positions
|
||||||
|
|
||||||
def count_macros(self, content: str) -> dict:
|
def count_macros(self, content: str) -> dict:
|
||||||
@@ -211,4 +234,4 @@ class MacroParser:
|
|||||||
Returns:
|
Returns:
|
||||||
True if any macros found
|
True if any macros found
|
||||||
"""
|
"""
|
||||||
return bool(self.MACRO_PATTERN.search(content))
|
return bool(self.MACRO_PATTERN.search(content) or self.SHORTHAND_PATTERN.search(content))
|
||||||
|
|||||||
@@ -259,3 +259,32 @@ class TestSQLiteArtifactRepository:
|
|||||||
assert retrieved.metadata.author == "test-author"
|
assert retrieved.metadata.author == "test-author"
|
||||||
assert retrieved.metadata.version == "1.0.0"
|
assert retrieved.metadata.version == "1.0.0"
|
||||||
assert retrieved.metadata.custom == {"key": "value"}
|
assert retrieved.metadata.custom == {"key": "value"}
|
||||||
|
|
||||||
|
def test_content_round_trip(self, repository):
|
||||||
|
"""Test that artifact content survives store and retrieve."""
|
||||||
|
original_content = "# Test Content\n\nThis is the full content."
|
||||||
|
artifact = Artifact.create(
|
||||||
|
space_id="test-space",
|
||||||
|
name="content-test",
|
||||||
|
content=original_content,
|
||||||
|
)
|
||||||
|
|
||||||
|
repository.create(artifact)
|
||||||
|
retrieved = repository.get_by_id(artifact.id)
|
||||||
|
|
||||||
|
assert retrieved.content == original_content
|
||||||
|
|
||||||
|
def test_content_persisted_after_update(self, repository):
|
||||||
|
"""Test that updated content is persisted."""
|
||||||
|
artifact = Artifact.create(
|
||||||
|
space_id="test-space",
|
||||||
|
name="update-test",
|
||||||
|
content="Original content",
|
||||||
|
)
|
||||||
|
repository.create(artifact)
|
||||||
|
|
||||||
|
artifact.update_content("Updated content")
|
||||||
|
repository.update(artifact)
|
||||||
|
|
||||||
|
retrieved = repository.get_by_id(artifact.id)
|
||||||
|
assert retrieved.content == "Updated content"
|
||||||
|
|||||||
@@ -97,7 +97,7 @@ class TestContextCompiler:
|
|||||||
|
|
||||||
# Macro should be replaced with resolved content
|
# Macro should be replaced with resolved content
|
||||||
assert "{{require:intro}}" not in compiled.content
|
assert "{{require:intro}}" not in compiled.content
|
||||||
assert "[Content of intro from space-1]" in compiled.content
|
assert "Introduction text" in compiled.content
|
||||||
assert "intro" in compiled.dependency_digests
|
assert "intro" in compiled.dependency_digests
|
||||||
|
|
||||||
def test_compile_with_optional_macros_substitutes_empty(
|
def test_compile_with_optional_macros_substitutes_empty(
|
||||||
@@ -126,7 +126,7 @@ class TestContextCompiler:
|
|||||||
|
|
||||||
# Optional missing macro should be removed
|
# Optional missing macro should be removed
|
||||||
assert "{{optional:missing}}" not in compiled.content
|
assert "{{optional:missing}}" not in compiled.content
|
||||||
assert compiled.content == "Start [Content of present from space-1] middle end"
|
assert compiled.content == "Start Present content middle end"
|
||||||
|
|
||||||
def test_compile_failed_resolution_raises_error(
|
def test_compile_failed_resolution_raises_error(
|
||||||
self, compiler, analyzer, resolver
|
self, compiler, analyzer, resolver
|
||||||
@@ -257,3 +257,32 @@ class TestContextCompiler:
|
|||||||
assert info["dependency_count"] == 1
|
assert info["dependency_count"] == 1
|
||||||
assert "dep" in info["dependencies"]
|
assert "dep" in info["dependencies"]
|
||||||
assert info["is_partial"] is False
|
assert info["is_partial"] is False
|
||||||
|
|
||||||
|
def test_compile_with_programmatic_macros_no_corruption(
|
||||||
|
self, compiler, analyzer, resolver, artifact_service
|
||||||
|
):
|
||||||
|
"""Test that compilation with programmatic macros doesn't corrupt content."""
|
||||||
|
artifact_service.create_artifact(
|
||||||
|
space_id="space-1",
|
||||||
|
name="dep",
|
||||||
|
content="Dependency content",
|
||||||
|
)
|
||||||
|
|
||||||
|
# Use @{target} shorthand syntax (parsed into programmatic macros)
|
||||||
|
content = "Before @{dep} after"
|
||||||
|
template = PromptTemplate.create(
|
||||||
|
space_id="space-1",
|
||||||
|
name="test",
|
||||||
|
content=content,
|
||||||
|
)
|
||||||
|
analyzer.analyze(template, content)
|
||||||
|
|
||||||
|
config = ResolutionConfig(space_id="space-1")
|
||||||
|
result = resolver.resolve_template(template, config)
|
||||||
|
|
||||||
|
compiled = compiler.compile(template, content, result)
|
||||||
|
|
||||||
|
# Content should be cleanly substituted, not corrupted
|
||||||
|
assert "@{dep}" not in compiled.content
|
||||||
|
assert "Dependency content" in compiled.content
|
||||||
|
assert compiled.content == "Before Dependency content after"
|
||||||
|
|||||||
@@ -177,3 +177,66 @@ class TestMacroParser:
|
|||||||
content = "{{generate:test| key = value }}"
|
content = "{{generate:test| key = value }}"
|
||||||
macros = self.parser.parse(content)
|
macros = self.parser.parse(content)
|
||||||
assert macros[0].parameters == {"key": "value"}
|
assert macros[0].parameters == {"key": "value"}
|
||||||
|
|
||||||
|
# --- Shorthand @{target} syntax tests ---
|
||||||
|
|
||||||
|
def test_parse_shorthand_macro(self):
|
||||||
|
"""Test parsing @{target} shorthand syntax."""
|
||||||
|
content = "Some text @{glossary} more text"
|
||||||
|
macros = self.parser.parse(content)
|
||||||
|
|
||||||
|
assert len(macros) == 1
|
||||||
|
assert macros[0].kind == MacroKind.REQUIRED
|
||||||
|
assert macros[0].target == "glossary"
|
||||||
|
assert macros[0].raw_text == "@{glossary}"
|
||||||
|
assert macros[0].parameters == {}
|
||||||
|
|
||||||
|
def test_parse_shorthand_with_line_numbers(self):
|
||||||
|
"""Test shorthand macros record line numbers."""
|
||||||
|
content = "Line 1\n@{dep1}\nLine 3\n@{dep2}"
|
||||||
|
macros = self.parser.parse(content)
|
||||||
|
|
||||||
|
assert len(macros) == 2
|
||||||
|
assert macros[0].line_number == 2
|
||||||
|
assert macros[1].line_number == 4
|
||||||
|
|
||||||
|
def test_parse_mixed_syntax(self):
|
||||||
|
"""Test parsing both {{kind:target}} and @{target} in same content."""
|
||||||
|
content = "{{require:dep1}} and @{dep2} and {{optional:dep3}}"
|
||||||
|
macros = self.parser.parse(content)
|
||||||
|
|
||||||
|
assert len(macros) == 3
|
||||||
|
assert macros[0].kind == MacroKind.REQUIRED
|
||||||
|
assert macros[0].target == "dep1"
|
||||||
|
assert macros[1].kind == MacroKind.OPTIONAL
|
||||||
|
assert macros[1].target == "dep3"
|
||||||
|
assert macros[2].kind == MacroKind.REQUIRED
|
||||||
|
assert macros[2].target == "dep2"
|
||||||
|
|
||||||
|
def test_shorthand_has_macros(self):
|
||||||
|
"""Test has_macros returns True for shorthand syntax."""
|
||||||
|
assert self.parser.has_macros("@{target}") is True
|
||||||
|
assert self.parser.has_macros("no macros here") is False
|
||||||
|
|
||||||
|
def test_shorthand_count_macros(self):
|
||||||
|
"""Test count_macros includes shorthand macros."""
|
||||||
|
content = "@{dep1} @{dep2} {{optional:dep3}}"
|
||||||
|
counts = self.parser.count_macros(content)
|
||||||
|
assert counts['required'] == 2
|
||||||
|
assert counts['optional'] == 1
|
||||||
|
|
||||||
|
def test_shorthand_find_macro_positions(self):
|
||||||
|
"""Test find_macro_positions includes shorthand macros."""
|
||||||
|
content = "Start @{dep1} middle {{require:dep2}} end"
|
||||||
|
positions = self.parser.find_macro_positions(content)
|
||||||
|
|
||||||
|
assert len(positions) == 2
|
||||||
|
# Should be sorted by position
|
||||||
|
assert positions[0][2] == "@{dep1}"
|
||||||
|
assert positions[1][2] == "{{require:dep2}}"
|
||||||
|
|
||||||
|
def test_shorthand_target_trimmed(self):
|
||||||
|
"""Test shorthand target with spaces is trimmed."""
|
||||||
|
content = "@{ my-artifact }"
|
||||||
|
macros = self.parser.parse(content)
|
||||||
|
assert macros[0].target == "my-artifact"
|
||||||
|
|||||||
@@ -32,6 +32,23 @@ class TestContentMacro:
|
|||||||
)
|
)
|
||||||
assert macro.parameters == {"language": "python", "framework": "fastapi"}
|
assert macro.parameters == {"language": "python", "framework": "fastapi"}
|
||||||
|
|
||||||
|
def test_programmatic_macro_gets_auto_derived_raw_text(self):
|
||||||
|
"""Test that programmatically-built macro gets auto-derived raw_text."""
|
||||||
|
macro = ContentMacro(
|
||||||
|
kind=MacroKind.REQUIRED,
|
||||||
|
target="glossary",
|
||||||
|
)
|
||||||
|
assert macro.raw_text == "@{glossary}"
|
||||||
|
|
||||||
|
def test_explicit_raw_text_not_overridden(self):
|
||||||
|
"""Test that explicit raw_text is preserved."""
|
||||||
|
macro = ContentMacro(
|
||||||
|
kind=MacroKind.REQUIRED,
|
||||||
|
target="glossary",
|
||||||
|
raw_text="{{require:glossary}}",
|
||||||
|
)
|
||||||
|
assert macro.raw_text == "{{require:glossary}}"
|
||||||
|
|
||||||
def test_macro_str_representation(self):
|
def test_macro_str_representation(self):
|
||||||
"""Test string representation."""
|
"""Test string representation."""
|
||||||
macro = ContentMacro(
|
macro = ContentMacro(
|
||||||
|
|||||||
Reference in New Issue
Block a user