-
Notifications
You must be signed in to change notification settings - Fork 8
Add optional protobuf response information in pythonic models with parameter validation #810
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
Conversation
Co-authored-by: zeiler <2138258+zeiler@users.noreply.github.com>
Co-authored-by: zeiler <2138258+zeiler@users.noreply.github.com>
Co-authored-by: zeiler <2138258+zeiler@users.noreply.github.com>
clarifai/client/model_client.py
Outdated
| def bind_f(method_name, method_argnames, call_func, async_call_func): | ||
| def sync_f(*args, **kwargs): | ||
| # Extract with_proto parameter if present | ||
| with_proto = kwargs.pop('with_proto', False) |
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.
can we ad "with_proto" as a constant and then in ModelClass verify that a user provided method does not have that argument name when they subclass ModelClass. That vailidation should be in method_signatures.py.
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.
Added RESERVED_PARAM_WITH_PROTO constant in method_signatures.py and validation in build_function_signature() to prevent user methods from using this reserved parameter name. ModelClient now uses the constant instead of hardcoded strings. Commit: 3b8ff57
clarifai/client/model_client.py
Outdated
| proto = resources_pb2.Input() | ||
| serialize({stream_argname: item}, [stream_sig], proto.data) | ||
| yield proto | ||
| # subsequent items are just the stream items |
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.
why did this get indented?
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.
Fixed the indentation issue - the "subsequent items are just the stream items" comment and async for loop were incorrectly indented inside the _input_proto_stream function. They should be at the module level. Commit: 3b8ff57
|
@copilot please address comment.s |
Co-authored-by: zeiler <2138258+zeiler@users.noreply.github.com>
Minimum allowed line rate is |
…rameter validation (#810) * Initial plan * Implement with_proto functionality in ModelClient for pythonic models Co-authored-by: zeiler <2138258+zeiler@users.noreply.github.com> * Add demo and finalize with_proto feature implementation Co-authored-by: zeiler <2138258+zeiler@users.noreply.github.com> * Address code review feedback and finalize implementation Co-authored-by: zeiler <2138258+zeiler@users.noreply.github.com> * Add with_proto constant and validation, fix indentation issue Co-authored-by: zeiler <2138258+zeiler@users.noreply.github.com> * lint --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: zeiler <2138258+zeiler@users.noreply.github.com> Co-authored-by: Matthew Zeiler <zeiler@clarifai.com>
Problem
When using pythonic models (subclasses of ModelClass), users could only access the processed/deserialized response from method calls like
predict(),generate(), etc. However, the underlying protobuf API response contains valuable additional information such as:Users requested the ability to optionally access this underlying protobuf response while maintaining backward compatibility.
Solution
This PR adds support for an optional
with_proto=True/Falseparameter to all pythonic model methods. When enabled, methods return both the processed result and the raw protobuf response as a tuple.Key Features
with_protodefaults toFalsegenerate()andstream()with_protoparameter nameUsage Examples
Parameter Name Protection
The framework now validates that user methods cannot use the reserved parameter name:
Implementation Details
Core Changes
bind_f()to extract and handle thewith_protoparameter before argument validation_predict,_generate,_streammethods (both sync and async versions) now acceptwith_protoparameterwith_proto=True, methods return(result, proto_response)tuple; otherwise return just the resultRESERVED_PARAM_WITH_PROTOconstant and validation inbuild_function_signature()to prevent conflictsFiles Modified
clarifai/client/model_client.py: Core implementation and constant usageclarifai/runners/utils/method_signatures.py: Parameter validation and constant definitiontests/test_with_proto_feature.py: Comprehensive test suite (14 tests)examples/with_proto_demo.py: Usage examples and documentationBackward Compatibility
The implementation is fully backward compatible:
with_proto=False) unchangedTesting & Quality
This enhancement provides powerful debugging and inspection capabilities while maintaining the clean, pythonic interface that users expect and preventing parameter name conflicts through proactive validation.
Original prompt
<issue_description>When calling a pythonic model that is a sublcass of ModelClass we allow using the model_client.py code to call the function directly. So if the subclass of ModelClass has a method predict(prompt, reasoning_effort) then the client side can do:
We would like to allow any of those methods to accept an additional parameter like
with_proto=True/Falsethat would change the client side function call to return two pieces of information: 1) the current response as it does today and 2) additional details we have from the underlying protobuf by returning the underlying proto that the API returns.</issue_description>Comments on the Issue (you are @copilot in this section)
Fixes #809
Original prompt
Fixes #809
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.