[FEATURE]: Validation engine

[grahamking]

The lib/runtime and lib/llm crates have a concept of AsyncEngine that is wrapped with other things, using lots of generics.

Make one of those wrappers be a layer that validates that the input is a valid OpenAI compliant request. Check the fields, check their ranges, maybe their types, all that. Respond with helpful error messages.

[nathan-barry]

Hey @grahamking I can start working on this. If you could provide a little bit more context and specifics that would be nice. Regardless, I'll look into it. Thanks

[grahamking]

This one is a bit more complex.

An engine is something that implements AsyncEngine for the appropriate generics. Here is an example: https://github.com/ai-dynamo/dynamo/blob/main/lib/llm/src/engines.rs#L221-L227 (there are others in that file, look at them too).

The idea is that we write an engine implementation with the same signature as EchoEngineFull (for completions, and a separate one for chat completions), which wraps another engine. It looks at various fields in NvCreateChatCompletionRequest and NvCreateCompletionRequest, ensures they are reasonable, returns appropriate errors, and if everything is OK forwards to the engine it wraps.

This validation engine would probably go in the lib/llm/src/engines.rs file.

That means all our engines can assume valid inputs. We centralize validation in one place.

As for what to validate I'm hoping there is something online with valid values for the various fields (temperature, top_p, frequency_penalty, etc), maybe in the OpenAI docs . You can also look at for example the mistralrs engine to see what it checks for: https://github.com/ai-dynamo/dynamo/blob/main/lib/engines/mistralrs/src/lib.rs#L269

[nathan-barry]

Alright, I've looked at the code a little tonight and here are my initial thoughts:

• The struct NvCreateCompletionRequest consists of async_openai::types::CreateCompletionRequest and Option<NvExt> (similar with the chat version).
• It looks like NvExt is already validating its inputs via #[derive(Validate)] (still need to look into how this works) and adding #[validate(...)] macro on the relevant fields. The async_openai types don't have this, so we'd need to add a method that manually does this.

Both CreateCompletionRequest and CreateChatCompletionRequest have comments after fields that require specific values (ex: pub temperature: Option<f32>, // min: 0, max: 2, default: 1). Here's the OpenAI API Reference, I'll double-check to see if any constraints are missing. Many of these constraints / magic nubmers are already defined in lib/llm/src/protocols/openai.rs (ex., MAX_TOP_P). Both structs have a lot of overlap in fields.

My initial plan is:

• Add lib/llm/src/protocols/openai/validate.rs that implements a bunch of helpers for validation (similar to this file in this library here)
• Add a validate_input method to NvCreateCompletionRequest and NvCreateChatCompletionRequest, which returns a result
• Implement ValidateEngine with essentially all the AsyncEngine generic combinations that are implemented for EchoEngineFull already. I believe the code will look pretty similar, but with just a validate_input method called and error handling, right? Also, I presume I only need to validate the requests, not the responses?

Let me know if this sounds like a sane implementation or if there's anything I'm missing or things that would be good to know / idomatic for the codebase. If this is more or less good I'll add this sometime in the next few days when I have time. Thanks.

[grahamking]

Sounds like a good plan.

Could you start with the ValidateEngine implementation, and maybe make it validate a single thing (e.g. temperature), and put up a PR for that. That's the more complex piece so I'd like to review that first and separately.

Then we can have the bulk of the actual validations in a separate PR afterwards.

• Further notes:

NvExt doesn't really matter for validation (it's tiny), it's the OpenAI type that's important.

with just a validate_input method called and error handling, right?

Exactly.

I presume I only need to validate the requests, not the responses?

Correct.

[grahamking]

Thanks @nathan-barry . Done here: #1674