Skip to content

Commit ea6c3d2

Browse files
jacoblee93jacoblee93
authored
fix: Fix format detection when pulling legacy prompts (#115)
* Fix format detection when pulling legacy prompts * Feedback
1 parent 1f8e4f0 commit ea6c3d2

5 files changed

Lines changed: 160 additions & 3 deletions

File tree

‎AGENTS.md‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,25 @@ The project uses ktfmt with `--kotlinlang-style`.
157157

158158
## Testing
159159

160+
### Don't add comments that restate the test name
161+
162+
Test function names should be descriptive enough on their own. Don't add comments that repeat what the name already says:
163+
164+
```kotlin
165+
// Good — name is self-documenting
166+
@Test
167+
fun parseLegacyPromptTemplateWithTemplateFormat() {
168+
val manifest = ...
169+
}
170+
171+
// Bad — comment restates the function name
172+
@Test
173+
fun parseLegacyPromptTemplateWithTemplateFormat() {
174+
// Legacy PromptTemplate format with template_format field
175+
val manifest = ...
176+
}
177+
```
178+
160179
### Running tests
161180

162181
```bash
@@ -189,4 +208,25 @@ Tests skip gracefully via `assumeTrue` if keys are missing.
189208

190209
- `toString()` should be single-line, following the `ClassName{field=value, field=value}` convention used by the rest of the SDK.
191210
- Avoid `@Suppress("UNCHECKED_CAST")` — restructure code to use safe patterns (`as? String`, `is Map<*, *>` with `entries.associate`, etc).
211+
- Use named arguments for constructor/function calls with 2+ parameters, especially when types could be confused:
212+
```kotlin
213+
// Good
214+
PromptMessage(
215+
role = PromptMessage.Role.HUMAN,
216+
template = template,
217+
templateFormat = templateFormat,
218+
)
219+
220+
// Bad — positional args are ambiguous
221+
PromptMessage(PromptMessage.Role.HUMAN, template, templateFormat = templateFormat)
222+
```
223+
- When an `Optional` has a fallback default, use `orElse(default)` directly instead of `orElse(null) ?: default`:
224+
```kotlin
225+
// Good — default goes straight into orElse
226+
kwargs["template_format"]?.asString()?.orElse("f-string") ?: "f-string"
227+
228+
// Bad — creates unnecessary null intermediary
229+
kwargs["template_format"]?.asString()?.orElse(null) ?: "f-string"
230+
```
231+
The `?: "f-string"` is still needed to handle the case where the key is missing from the map (`null` from `kwargs["template_format"]`), but `orElse` should carry the default for when the key exists but isn't a string.
192232
- Anthropic SDK is a `compileOnly` dependency — users must add it themselves. Methods that use Anthropic types should catch `NoClassDefFoundError` and throw `IllegalStateException` with a clear message.

‎langsmith-java-core/src/main/kotlin/com/langchain/smith/prompts/ManifestParser.kt‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -102,9 +102,17 @@ internal object ManifestParser {
102102
kwargs["template"]?.asString()?.orElse(null)
103103
?: throw IllegalArgumentException("PromptTemplate missing 'template' in kwargs")
104104

105+
val templateFormat = kwargs["template_format"]?.asString()?.orElse("f-string") ?: "f-string"
105106
val inputVariables = extractInputVariables(kwargs)
106107

107-
return PromptMessages(listOf(PromptMessage.human(template)), inputVariables)
108+
return PromptMessages.of(
109+
PromptMessage(
110+
role = PromptMessage.Role.HUMAN,
111+
template = template,
112+
templateFormat = templateFormat,
113+
),
114+
inputVariables,
115+
)
108116
}
109117

110118
/** Holds a parsed template string and its format. */
@@ -164,15 +172,15 @@ internal object ManifestParser {
164172
private fun extractTemplate(kwargs: Map<String, JsonValue>): TemplateInfo? {
165173
// Check for direct template
166174
kwargs["template"]?.asString()?.orElse(null)?.let { tmpl ->
167-
val fmt = kwargs["template_format"]?.asString()?.orElse(null) ?: "f-string"
175+
val fmt = kwargs["template_format"]?.asString()?.orElse("f-string") ?: "f-string"
168176
return TemplateInfo(tmpl, fmt)
169177
}
170178

171179
// Check for nested prompt object
172180
val prompt = kwargs["prompt"]?.asObject()?.orElse(null) ?: return null
173181
val promptKwargs = extractKwargs(prompt)
174182
val tmpl = promptKwargs["template"]?.asString()?.orElse(null) ?: return null
175-
val fmt = promptKwargs["template_format"]?.asString()?.orElse(null) ?: "f-string"
183+
val fmt = promptKwargs["template_format"]?.asString()?.orElse("f-string") ?: "f-string"
176184
return TemplateInfo(tmpl, fmt)
177185
}
178186

‎langsmith-java-core/src/main/kotlin/com/langchain/smith/prompts/PromptMessages.kt‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,13 @@ internal class PromptMessages(
114114
}
115115
return "PromptMessages{${parts.joinToString(", ")}}"
116116
}
117+
118+
companion object {
119+
120+
/** Creates a [PromptMessages] from a single message and input variables. */
121+
fun of(message: PromptMessage, inputVariables: List<String>): PromptMessages =
122+
PromptMessages(listOf(message), inputVariables)
123+
}
117124
}
118125

119126
private fun toPromptMessage(value: Any?): PromptMessage? =

‎langsmith-java-core/src/test/kotlin/com/langchain/smith/prompts/ManifestParserTest.kt‎

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,32 @@ internal class ManifestParserTest {
114114
assertThat(result.inputVariables).containsExactly("topic")
115115
}
116116

117+
@Test
118+
fun parseLegacyPromptTemplateWithTemplateFormat() {
119+
val manifest =
120+
mapOf(
121+
"lc" to 1,
122+
"type" to "constructor",
123+
"id" to listOf("langchain_core", "prompts", "prompt", "PromptTemplate"),
124+
"kwargs" to
125+
mapOf(
126+
"input_variables" to listOf("input"),
127+
"template_format" to "f-string",
128+
"template" to
129+
"You are a parrot. The current date is 2026-03-29T14:26:33.834Z\n{input}",
130+
),
131+
)
132+
133+
val result = ManifestParser.parse(JsonValue.from(manifest))
134+
135+
assertThat(result.messages).hasSize(1)
136+
assertThat(result.messages[0].role).isEqualTo(PromptMessage.Role.HUMAN)
137+
assertThat(result.messages[0].template)
138+
.isEqualTo("You are a parrot. The current date is 2026-03-29T14:26:33.834Z\n{input}")
139+
assertThat(result.messages[0].templateFormat).isEqualTo("f-string")
140+
assertThat(result.inputVariables).containsExactly("input")
141+
}
142+
117143
@Test
118144
fun parseDirectTemplateInKwargs() {
119145
// Some manifests have the template directly in the message kwargs,
@@ -144,6 +170,41 @@ internal class ManifestParserTest {
144170
assertThat(result.messages[0].template).isEqualTo("Direct template text")
145171
}
146172

173+
@Test
174+
fun parseDirectTemplateWithTemplateFormat() {
175+
// Test that template_format is preserved in direct template pattern
176+
val manifest =
177+
buildChatPromptManifest(
178+
messages =
179+
listOf(
180+
mapOf(
181+
"lc" to 1,
182+
"type" to "constructor",
183+
"id" to
184+
listOf(
185+
"langchain_core",
186+
"prompts",
187+
"chat",
188+
"HumanMessagePromptTemplate",
189+
),
190+
"kwargs" to
191+
mapOf(
192+
"template" to "Hello {{name}}",
193+
"template_format" to "mustache",
194+
),
195+
)
196+
),
197+
inputVariables = listOf("name"),
198+
)
199+
200+
val result = ManifestParser.parse(JsonValue.from(manifest))
201+
202+
assertThat(result.messages).hasSize(1)
203+
assertThat(result.messages[0].role).isEqualTo(PromptMessage.Role.HUMAN)
204+
assertThat(result.messages[0].template).isEqualTo("Hello {{name}}")
205+
assertThat(result.messages[0].templateFormat).isEqualTo("mustache")
206+
}
207+
147208
@Test
148209
fun parseManifestWithoutExplicitId() {
149210
// Falls back to detecting type from kwargs structure

‎langsmith-java-core/src/test/kotlin/com/langchain/smith/prompts/PromptTest.kt‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,4 +66,45 @@ internal class PromptTest {
6666
assertThat(prompt.hasOutputSchema()).isFalse()
6767
assertThat(prompt.outputSchema).isNull()
6868
}
69+
70+
@Test
71+
fun legacyPromptTemplateEndToEnd() {
72+
val prompt =
73+
promptFromManifest(
74+
mapOf(
75+
"lc" to 1,
76+
"type" to "constructor",
77+
"id" to listOf("langchain_core", "prompts", "prompt", "PromptTemplate"),
78+
"kwargs" to
79+
mapOf(
80+
"input_variables" to listOf("input"),
81+
"template_format" to "f-string",
82+
"template" to
83+
"You are a parrot. The current date is 2026-03-29T14:26:33.834Z\n{input}",
84+
),
85+
)
86+
)
87+
88+
assertThat(prompt.inputVariables).containsExactly("input")
89+
90+
val result = prompt.invoke(mapOf("input" to "Hello world!"))
91+
92+
assertThat(result.messages).hasSize(1)
93+
assertThat(result.messages[0].role).isEqualTo(PromptMessage.Role.HUMAN)
94+
assertThat(result.messages[0].template)
95+
.isEqualTo(
96+
"You are a parrot. The current date is 2026-03-29T14:26:33.834Z\nHello world!"
97+
)
98+
}
99+
100+
/** Creates a [Prompt] from a raw manifest map, simulating the pull → parse flow. */
101+
private fun promptFromManifest(manifest: Map<String, Any>): Prompt =
102+
Prompt.fromCommit(
103+
PromptCommit.of(
104+
owner = "test",
105+
repo = "test",
106+
commitHash = "test",
107+
manifest = com.langchain.smith.core.JsonValue.from(manifest),
108+
)
109+
)
69110
}

0 commit comments

Comments
 (0)