-
Notifications
You must be signed in to change notification settings - Fork 23
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
✨🐛 Pass on non-threshold detector parameters #235
✨🐛 Pass on non-threshold detector parameters #235
Conversation
Signed-off-by: Evaline Ju <[email protected]>
Signed-off-by: Evaline Ju <[email protected]>
Signed-off-by: Evaline Ju <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, overall. Left a few questions.
Co-authored-by: Mateus Devino <[email protected]> Signed-off-by: Evaline Ju <[email protected]>
Signed-off-by: Evaline Ju <[email protected]>
Signed-off-by: Evaline Ju <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Signed-off-by: Evaline Ju <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me!
) * ✨ Pass along detector_params for text contents and generation Signed-off-by: Evaline Ju <[email protected]> * ✨✅ Not pass on threshold param Signed-off-by: Evaline Ju <[email protected]> * 🐛 Mutable threshold not intuitive Signed-off-by: Evaline Ju <[email protected]> * Update src/clients/detector/text_chat.rs Co-authored-by: Mateus Devino <[email protected]> Signed-off-by: Evaline Ju <[email protected]> * 💡🎨 Update params comment and remove unnecessary clones Signed-off-by: Evaline Ju <[email protected]> * ✅ Threshold re-call not problematic Signed-off-by: Evaline Ju <[email protected]> --------- Signed-off-by: Evaline Ju <[email protected]> Co-authored-by: Mateus Devino <[email protected]> Signed-off-by: resoluteCoder <[email protected]>
detector_params
to additional detectors API endpoints (/text/contents
and/text/generation
) as documented through 🔧 Add detector_params as optional for every detectors endpoint #212threshold
in the orchestrator, so we do not want to pass this on to the detectors. This is currently just removed. This now builds off 🐛 Fix threshold getting passed through beyond orchestrator proces… #236 which assumespop_threshold
will only be done once per detector flowCloses: #227 , #234