fix(go): capture local template in Compile closure to prevent sharing#363
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
|
I have signed the CLA. |
The Compile() method was storing the parsed template in dp.Template and referencing it in the returned closure. When multiple prompts were compiled on the same Dotprompt instance, all closures would share the same dp.Template, causing subsequent prompts to use the wrong template. This fix captures the current template in a local variable before creating the closure, ensuring each compiled PromptFunction uses its own template. Fixes google#362
175e440 to
21db81b
Compare
Test ResultsI tested this fix with the memory project using replace github.com/google/dotprompt/go => /tmp/dotprompt/goUnit test in dotprompt package: ✅ PASS Note: The fix resolves the template sharing issue when compiling multiple different prompts on the same |
Summary
Compile()method where all compiledPromptFunctionclosures shared the samedp.TemplateTestCompileMultiplePromptsTemplateIsolationto prevent future regressionsProblem
When using the
Compile()method multiple times on the sameDotpromptinstance, all compiledPromptFunctionclosures share the samedp.Templatefield. This causes subsequent prompt executions to use the wrong template.Root Cause
In
dotprompt.go, theCompilemethod:renderTplfrom parsing the sourcedp.Templatefield viainitializeTemplate()renderFuncthat referencesdp.TemplateWhen another prompt is compiled,
dp.Templategets overwritten, causing all previousrenderFuncclosures to use the wrong template.Example
Solution
Capture the current template in a local variable before creating the closure:
Test Plan
TestCompileMultiplePromptsTemplateIsolationregression testFixes #362