fix: do not attempt to deserialize llm text response#233
Conversation
eric-tramel
left a comment
There was a problem hiding this comment.
IIUC, one doesn't need to do this in the recipe itself because the text recipe is a string pass-through anyway. However, you introduce the true JSON deserialization conditionally in the case of structured & judge because we require structured output in these cases.
In the ol' days we were forcibly deserializing columns as we processed, are we sure that serialized string columns stay serialized? E.g. perhaps this is fine for one column, but if the next one attempts to eagerly deserialize everything that can be deserialized, do we have any problems with it getting updated back into the dataset deserialized, or do we now have contract that the only way that entries can get mutated in the dataset is on their generation call? This wasn't the case previously, so I just want to be sure.
Oh yea good point. If the next column references this column, this line will try to deserialized the nested deserializable string in the dictionary container for the row. At that point the next column will overwrite the value for that initial column with a deserialized json object, right? |
Actually, the next column doesn't have to reference this column at all. Just the fact that there's another generation column would make the above true. |
Tracing though, while we do deserialize the dictionary with values from previous columns here we don't modify the original data container dictionary with the deserialized values. The deserialized values are only used for template rendering... so shouldn't have any side effect. |
Nice digging @nabinchha! Might be worth adding some comments (maybe in the |
good call. Updated the docstring a bit + made sure data passed into |
closes: #232