Handle arrays of images when building prompts #1247
Conversation
cpinn
force-pushed
the
caitlin/modify-nunjucks-parsing-to-match-custom-mustache-behavior
branch
from
97a2bde to
74e0bc1
Compare
js/src/template/nunjucks-env.ts
Outdated
| throwOnUndefined, | ||
| }); | ||
|
|
||
| // Intercept renderString to wrap objects with custom toString |
There was a problem hiding this comment.
nunjucks doesn't really support the type of custom escaping we were doing for mustache. This implements some of the custom parsing we were doing just to make sure that [Object object] doesn't get printed out during the toString method. Kind of hacky but it works to match the behavior.
cpinn
force-pushed
the
caitlin/modify-nunjucks-parsing-to-match-custom-mustache-behavior
branch
from
77b2ee2 to
d6a79e6
Compare
| return new nunjucks.Environment(null, { | ||
| autoescape: true, | ||
| const env = new nunjucks.Environment(null, { | ||
| autoescape: false, |
There was a problem hiding this comment.
i'd double check our docs to make sure we call out that we don't escape by default in both template renderers. perhaps how to escape, for those that want to
There was a problem hiding this comment.
We don't call this out at all, never did which is why I was quite surprised by it.
js/src/template/renderer.test.ts
Outdated
| expect(result).toBe( | ||
| 'User: {"name":"Alice","age":30}, Items: ["a","b","c"]', | ||
| ); | ||
| expect(result).not.toContain("[object Object]"); |
There was a problem hiding this comment.
i'd encourage a literal comparison, if possible. the not.toContain could be misleading since any value is allowed except "[object ...
There was a problem hiding this comment.
The actual comparison is just in the line above
expect(result).toBe(
'User: {"name":"Alice","age":30}, Items: ["a","b","c"]',
);
js/src/template/renderer.test.ts
Outdated
| { templateFormat: "nunjucks" }, | ||
| ); | ||
| // Arrays output as comma-separated (Nunjucks default behavior) | ||
| expect(resultWithAccess).toBe("User: Bob, Age: 25, Items: 1,2,3"); |
There was a problem hiding this comment.
i expected the array to be ["1", "2", "3"] 🤔 (json stringify)
There was a problem hiding this comment.
I haven't decided if I want to overwrite this or not because this is just the behavior of nunjucks.
if you have an array it doesn't get output as [object object] it just gets outputs the array as such, this is regular rendering. Its a different behavior than mustache above which would print out the array.
There was a problem hiding this comment.
looks like mustache does do as I'd expected. 🤔
| escape, | ||
| { templateFormat: "nunjucks" }, | ||
| ); | ||
| expect(result).toBe('Data: {"nested":{"value":123,"items":["x","y"]}}'); |
There was a problem hiding this comment.
I think what will be interesting is that nunjucks renders arrays by default as x,y but if a nested object is printed and that is determined to be an object it will use the custom parser that will json stringify the entire object.
cpinn
changed the title
cpinn
force-pushed
the
caitlin/modify-nunjucks-parsing-to-match-custom-mustache-behavior
branch
from
22803a9 to
5df5f53
Compare
Users would like to be able to reference an array when handling images.
This Pr does the following