Conversation
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
|
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+223
to
+229
| top = stack.pop() if stack else False | ||
| if top is True: | ||
| # First "}" closes the doubled JSON brace: emit its | ||
| # escaped pair, then reprocess the second "}". | ||
| out.append("}}") | ||
| i += 1 | ||
| continue |
Contributor
There was a problem hiding this comment.
}}, as in {"regex": "}}"}, this branch treats those characters as the object's closing braces. It adds a third brace inside the string and leaves the actual object close unescaped, so get_langchain_prompt() returns an invalid LangChain template.
Prompt To Fix With AI
This is a comment left during a code review.
Path: langfuse/model.py
Line: 223-229
Comment:
**Literal braces close JSON early** When a JSON string value contains literal `}}`, as in `{"regex": "}}"}`, this branch treats those characters as the object's closing braces. It adds a third brace inside the string and leaves the actual object close unescaped, so `get_langchain_prompt()` returns an invalid LangChain template.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes langfuse/langfuse#18131.
User impact: prompts containing compact nested JSON (e.g.
Example: {"a": {"b": 1}}) render malformed throughget_langchain_prompt()— the closing brace of each nesting level is silently dropped ({"a": {"b": 1}instead of{"a": {"b": 1}}), breaking downstream LangChain templates.Technical cause: in
BasePromptClient._escape_json_for_langchain(langfuse/model.py), the closing-brace branch treats every adjacent}}as a pre-escaped pair and leaves it untouched — it never consults the stack, so the stack entry pushed when the JSON{was doubled is never popped and the escaped}}for that brace is never emitted. Each}}run in deeper nesting loses one more brace.Fix: when a
}}pair is seen, pop the stack: if the top entry is a doubled JSON{(True), emit its escaped}}and reprocess the second}so it can close the next brace. Pre-escaped{{pairs push aNonemarker so their own}}is still left untouched (verified:{"name": "{{name}}"}still renders correctly).Verification:
'Example: {{"a": {{"b": 1}}'renders{"a": {"b": 1}(malformed). After: escaped'Example: {{"a": {{"b": 1}}}}'renders the original exactly.tests/unit/test_prompt_compilation.py(test_compact_nested_json_keeps_closing_braces,test_pre_escaped_placeholder_inside_nested_json), both passing against the fixed code via the realTextPromptClient+PromptTemplate.The PR should not merge until literal
}}inside a JSON string no longer produces an invalid LangChain template.Summary
The PR changes brace-stack handling to preserve closing braces in compact nested JSON and adds two prompt-compilation tests.
Reviews (1) · Last reviewed commit: "test(sdk-python): regression tests for n..."