Zhuoxi2000 commented on code in PR #908:
URL: https://github.com/apache/flink-agents/pull/908#discussion_r3590496397
##########
api/src/main/java/org/apache/flink/agents/api/prompt/Prompt.java:
##########
@@ -205,19 +207,40 @@ public String toString() {
return "LocalPrompt{" + "template=" + template + '}';
}
- /** Format template string with keyword arguments */
+ /** Matches a {placeholder}; the key may not itself contain braces. */
+ private static final Pattern PLACEHOLDER =
Pattern.compile("\\{([^{}]+)\\}");
+
+ /**
+ * Format a template string with keyword arguments.
+ *
+ * <p>The template is scanned once and each {@code {key}} placeholder
is replaced with its
+ * value (or left untouched if the key is absent). Substitution is
single-pass: text
+ * introduced by a substituted value is never itself re-interpreted as
a placeholder. This
+ * keeps the result independent of the {@code kwargs} iteration order
and prevents a
+ * caller-supplied value from expanding into another variable's value.
It mirrors the Python
+ * {@code SafeFormatter}.
+ */
private static String format(String template, Map<String, String>
kwargs) {
if (template == null) {
return "";
}
- String result = template;
- for (Map.Entry<String, String> entry : kwargs.entrySet()) {
- String placeholder = "{" + entry.getKey() + "}";
- String value = entry.getValue() != null ? entry.getValue() :
"";
- result = result.replace(placeholder, value);
+ Matcher matcher = PLACEHOLDER.matcher(template);
+ StringBuilder result = new StringBuilder();
+ while (matcher.find()) {
+ String key = matcher.group(1);
+ String replacement;
+ if (kwargs.containsKey(key)) {
+ String value = kwargs.get(key);
+ replacement = value != null ? value : "";
+ } else {
+ // Unknown placeholder: leave it as-is.
+ replacement = matcher.group(0);
+ }
+ matcher.appendReplacement(result,
Matcher.quoteReplacement(replacement));
Review Comment:
Thanks @weiqingy great catch you're right, I tried removing
`quoteReplacement`,it indeed makes the test fail it is doing important work
here. I've added the suggested Java test
I kept this Java-only, as you suggested, since Python's `re.sub` with a
function replacement doesn't have the same `$`/`\` behavior. Thanks for
catching this again!
Same failing check unrelated to this change
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
