Skip to content

feat: add an opt-in to leave escaped placeholders in instructions as written - #1569

Open
Laurianti wants to merge 1 commit into
google:mainfrom
Laurianti:fix-instruction-dollar-brace-placeholders
Open

Laurianti wants to merge 1 commit into
google:mainfrom
Laurianti:fix-instruction-dollar-brace-placeholders

Conversation

@Laurianti

@Laurianti Laurianti commented Sep 28, 2026 •

Copy link
Copy Markdown

Link to Issue or Description of Change

2. Or, if no issue exists, describe the change:

Ports the adk-python templating rule of google/adk-python@0caecd2 (google/adk-python#5706) as an opt-in: adk-python made it the default in 2.10.0 and listed it under behavior changes, while adk-java 1.x keeps today's output unless an agent opts in.

Problem:

InstructionUtils.injectSessionState resolves a placeholder right after $ or \ like any other and keeps the character in front, so instructions that show a template syntax cannot reach the model as written:

Template Output
The formatString supports interpolation via ${expression} syntax. IllegalArgumentException: Context variable not found
Workflow syntax: ${{expression}} (expression = foo) Workflow syntax: $foo
Price: ${price} (price = 9.99) Price: $9.99
A ${var?} B (var missing) A $ B
\{user_name} (user_name = Foo) \Foo

Solution:

LlmAgent.Builder.preserveEscapedPlaceholders(boolean), default false, lets an agent opt into the adk-python rule: a placeholder right after $ or \ is left as written, backslash included, so each template above reaches the model unchanged. Other placeholders, including {{key}} and optional ones, resolve as before.

  • LlmAgent: builder method, preserveEscapedPlaceholders() accessor, and preserve_escaped_placeholders in LlmAgentConfig for YAML agents.
  • InstructionUtils: the current pattern stays the default. The new injectSessionState(InvocationContext, String, boolean) selects the adk-python pattern; the existing two-argument method takes the flag from context.agent() when it is an LlmAgent, and false otherwise.
  • Instructions: the agent's flag applies to its instruction, the root agent's flag to the global instruction.
  • GlobalInstructionPlugin: a new GlobalInstructionPlugin(String, String, boolean) constructor. The existing constructors keep today's output whatever the current agent's flag.
  • The rule is documented in the Placeholder Syntax section of injectSessionState, in the templating paragraph of Instruction, on the builder method and in the GlobalInstructionPlugin class Javadoc.

An Instruction.Provider is not templated, so the flag does not affect it.

Testing Plan

Unit Tests:

  • I have added or updated unit tests for my change.

  • All unit tests pass locally.

  • InstructionUtilsTest: the four adk-python cases with the flag on, the escaped case as \{user_name} with state set, plus Price: ${price}, A ${var?} B, \{{user_name}}, and unescaped {key}, {{key}} and {key?} still resolving with the flag on. With the flag off, today's output is pinned: ${expression} throws, ${{expression}} renders $foo, Price: ${price} renders $9.99, A ${var?} B renders A $ B, \{user_name} renders \Foo. The two-argument method follows an opted-in LlmAgent, an LlmAgent with defaults, and a non-LlmAgent.

  • InstructionsTest: agent instruction with the flag on and off; global instruction following the root agent's flag, in both directions against the sub-agent's flag; a provider instruction left untouched.

  • GlobalInstructionPluginTest: the new constructor with the flag on and off and its name; both existing string constructors keeping today's output when the current agent opts in.

  • LlmAgentTest: default and opted-in builder, and a run of the agent checking the instruction sent to the model.

  • ConfigAgentUtilsTest: the YAML key set and absent.

  • AgentToolTest: an opted-in and a default agent wrapped in AgentTool.

Each production change fails at least one of these tests when reverted on its own (twelve single-change reverts checked). mvn -pl core -am test: 2051 tests, 0 failures, 0 errors, 24 skipped.

Manual End-to-End (E2E) Tests:

Covered by the LlmAgentTest and AgentToolTest runs above, which check the system instruction a test model receives.

Checklist

  • I have read the CONTRIBUTING.md document.
  • My pull request contains a single commit.
  • I have performed a self-review of my own code.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have added tests that prove my fix is effective or that my feature works.
  • New and existing unit tests pass locally with my changes.

@hemasekhar-p hemasekhar-p self-assigned this Sep 29, 2026
@hemasekhar-p

Copy link
Copy Markdown
Contributor

Hi @Laurianti, Thank you for your contribution and for taking the time to submit this pull request. Our team is currently reviewing your changes and we will reach out if we need any further information. Thank you.

@dosadczuk

Copy link
Copy Markdown

Thanks for the port and the clear write-up.

We cannot take this as-is on 1.x. adk-java 1.x does not ship behavior changes, and this changes the output of templates that work today:

Template Today With this PR
Price: ${price} (price = 9.99) Price: $9.99 Price: ${price}
A ${var?} B (var missing) A $ B A ${var?} B
\{user_name} (user_name = Foo) \Foo \{user_name}

None of these throw an error, so callers only notice when values stop reaching the model. adk-python made this change in 2.10.0 and listed it under behavior changes. For Java, we want this opt-in on 1.x, default in 2.0.

If you need this change, could you rework the PR to put the new parsing behind a flag? Here is what we need:

  • On LlmAgent, add Builder.preserveEscapedPlaceholders(boolean) defaulting to false, an accessor, and the matching LlmAgentConfig field for YAML agents. The name is open to suggestions.
  • Keep the current pattern as default in InstructionUtils. Add a 3-arg injectSessionState(InvocationContext, String, boolean preserveEscapedPlaceholders). Have the existing 2-arg method use context.agent()'s flag when it's an LlmAgent, leaving everyone else unchanged.
  • In Instructions, use the agent's flag for its own instruction and the root agent's flag for the global instruction.
  • Add a constructor to GlobalInstructionPlugin that accepts the flag, leaving existing constructors unchanged regardless of current agent.
  • Document the rule (a placeholder right after $ or \ is left as written, backslash included) in the injectSessionState Placeholder Syntax section, the Instruction templating paragraph, and the builder method. Callers cannot see the comment on the private field.
  • For tests, run your four cases with the flag on, and pin today's output with the flag off (${expression} throws, ${{expression}} renders $foo, Price: ${price} renders $9.99). Rewrite the escaped test case as \{user_name} with state set, because the current \{expression\} test passes without the fix. Add tests for the global instruction with the root flag, the plugin constructor, the YAML key, and an opted-in agent wrapped in AgentTool.
  • Retitle the PR with feat:.

NOTE: The listed changes might not be the only changes needed to make it work. If you decide to follow-up, make sure the impact of your change is fully covered.

@dosadczuk dosadczuk added waiting on reporter Waiting for reaction by reporter. Failing that, maintainers will eventually closed it as stale. needs update and removed needs review labels Sep 30, 2026
@dosadczuk
dosadczuk self-requested a review September 30, 2026 14:03
@Laurianti
Laurianti force-pushed the fix-instruction-dollar-brace-placeholders branch from fd023bd to 4cb1a5e Compare September 30, 2026 14:52
@Laurianti Laurianti changed the title fix: do not match dollar-brace or escaped patterns in instructions feat: add an opt-in to leave escaped placeholders in instructions as written Sep 30, 2026
@Laurianti

Laurianti commented Sep 30, 2026 •

Copy link
Copy Markdown
Author

Thanks for the table and the precise list. Reworked in 9928b62, still one commit:

  • LlmAgent: Builder.preserveEscapedPlaceholders(boolean), default false, the preserveEscapedPlaceholders() accessor, and preserve_escaped_placeholders in LlmAgentConfig. I kept the name.
  • InstructionUtils: the current pattern stays the default. The new injectSessionState(InvocationContext, String, boolean preserveEscapedPlaceholders) selects the adk-python one, and the 2-arg method uses the flag of context.agent() when it is an LlmAgent, false otherwise.
  • Instructions: the agent's flag for its own instruction, the root agent's flag for the global instruction.
  • GlobalInstructionPlugin: new GlobalInstructionPlugin(String globalInstruction, String name, boolean preserveEscapedPlaceholders). The existing string constructors pass false, so they ignore the current agent's flag.
  • Docs: the rule is in the Placeholder Syntax section of injectSessionState, the Instruction templating paragraph and the builder method, and also in the GlobalInstructionPlugin class Javadoc.
  • Tests: your four cases with the flag on; today's output pinned with the flag off (${expression} throws, ${{expression}} renders $foo, Price: ${price} renders $9.99, plus A ${var?} B and \{user_name}); the escaped case rewritten as \{user_name} with state set; the global instruction with the root flag, in both directions against the sub-agent's flag; the new and the existing plugin constructors; the YAML key; an opted-in agent wrapped in AgentTool. I also added a run of an opted-in LlmAgent that checks the instruction the model receives, and a check that {key}, {{key}} and {key?} still resolve with the flag on.
  • Title: now feat:.

On impact: InstructionUtils.injectSessionState has no other callers in the repository, including tokt, and an Instruction.Provider is not templated, so the flag does not affect it. Each production change fails at least one test when reverted on its own.

…written

`InstructionUtils.injectSessionState` resolves a placeholder right after `$` or
`\` like any other and keeps the character in front: `Price: ${price}` becomes
`Price: $9.99`, and `${expression}` fails the request when `expression` is not
in the state. adk-python leaves such placeholders as written since 2.10.0
(google/adk-python@0caecd2).

`LlmAgent.Builder.preserveEscapedPlaceholders(boolean)`, also available as
`preserve_escaped_placeholders` in YAML agent configs, lets an agent opt into the
same rule. The agent's flag applies to its instruction and the root agent's flag
to the global instruction; `GlobalInstructionPlugin` takes it as a constructor
argument. The default is unchanged.
@Laurianti
Laurianti force-pushed the fix-instruction-dollar-brace-placeholders branch from 4cb1a5e to 9928b62 Compare September 30, 2026 15:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs update waiting on reporter Waiting for reaction by reporter. Failing that, maintainers will eventually closed it as stale.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants