Update semantic model length validation for literal retrieval #72
Conversation
sfc-gh-nlimtiaco
requested review from
sfc-gh-nsehrawat,
sfc-gh-rehuang,
sfc-gh-jhilgart and
sfc-gh-dasilva
as code owners
| @@ -19,17 +72,40 @@ def validate_context_length(model: ProtoMsg, throw_error: bool = False) -> None: | |||
| """ | |||
|
|
|||
| model.ClearField("verified_queries") | |||
| _truncate_sample_values(model) | |||
| num_search_services = _count_search_services(model) | |||
|
|
|||
| yaml_str = proto_to_yaml(model) | |||
There was a problem hiding this comment.
not sure if this'll happen often: but i don't recall we strip any whitespaces during loading yaml/convert to proto; Shall we add stripping for whitespaces?
We can potentially add to proto field options like what Daniel had here: https://github.com/snowflakedb/cortex/pull/114221/files
There was a problem hiding this comment.
Not sure if whitespace is a problem, have you seen any issues with it? For yaml -> proto, I don't know of strictyaml not being robust to extra white space. For proto -> yaml, I don't see where we would output excess whitespace.
There was a problem hiding this comment.
yes, probably strictyaml handles that. Can you help add a todo at the top of this file just reminder to keep an eye on the white spaces?
Now that we can do literal retrieval based on sample values, we no longer will show all sample values to the model by default. Update the model length verification to take this into account.