feat(server): Align condition and template depth limits#166
feat(server): Align condition and template depth limits#166
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
lan17
changed the title
| type TemplateValue = str | bool | list[str] | ||
| type JsonValue = JSONValue | ||
|
|
||
| MAX_CONDITION_DEPTH = 12 |
There was a problem hiding this comment.
do you think a normal usecase would need this much depth? what was hte motivation behind doubling it?
There was a problem hiding this comment.
They might depending on how complex the Control is, which they might get complex if we generate them using LLMs (which i did for Leon The Controller).
IMO we should not dictate a shallow depth just because we are afraid users will misuse it. We should leave it up to them. We should have some depth that prevents some perf issues tho, so I would push this limit even higher in the future.
| with pytest.raises( | ||
| ValidationError, | ||
| match="Condition nesting depth exceeds maximum of 6", | ||
| match="Condition nesting depth exceeds maximum of 12", |
There was a problem hiding this comment.
can we use the constant MAX_CONDITION_DEPTH here instead?
2 participants
Summary
MAX_TEMPLATE_DEFINITION_DEPTHderive fromMAX_CONDITION_DEPTHso both limits stay alignedWhy
The condition-tree limit had been raised independently, but the template definition depth used a separate constant value. Referencing the shared constant removes drift and keeps template-backed controls aligned with the same supported nesting depth.
Impact
Template-backed and non-template control validation now share one source of truth for depth limits. Future changes to the condition limit will automatically apply to template definition depth as well.
Validation
make test TEST= models/tests/test_controls.py(this invoked the repo's full defaultmake testchain and passed across models, telemetry, server, engine, Python SDK, and builtin evaluators)make models-test