-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Documentation added for model handler #1067
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
base: main
Are you sure you want to change the base?
Conversation
super().__init__(model_name, temperature) | ||
self.model_style = ModelStyle.Anthropic | ||
self.client = Anthropic(api_key=os.getenv("ANTHROPIC_API_KEY")) | ||
|
||
def decode_ast(self, result, language="Python"): | ||
def decode_ast(self, result: str, language: str="Python") -> list[dict[str, dict]]: |
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.
result
is not guaranteed to be a string. When in prompting mode, it is indeed a string. However, when in FC mode, it would be a list of dictionaries; you can infer from lines 58-60 (cited below).
for invoked_function in result:
name = list(invoked_function.keys())[0]
params = json.loads(invoked_function[name])
Same issue in decode_execute
Pre-processes test data for function calling queries. | ||
|
||
Args: | ||
inference_data (dict): Inference data to process |
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.
The current description of inference_data
isn’t quite right.
-
Purpose of the function – It first pre-processes the given
test_entry
before sending it to the model, e.g.- converts the raw user message into the format the model expects
- extracts the system prompt, if one exists
- performs any other required data transformations
-
Role of
inference_data
– It’s simply a shared context dictionary passed between handler methods. This function only initialises that dict with a few default keys; it does not populate it with the processed dataset fields. -
Where the real work happens – All dataset-specific processing is done in-place on the
test_entry
dict. Nothing is currently copied intoinference_data
.
Hey @Aktsvigun , One thing I'm thinking: Because each concrete handler exposes the same methods with identical parameters and behavior, it might be cleaner to keep the detailed documentation in BaseHandler rather than duplicating it everywhere. IntelliSense should be able to surface the parent docstring when we hover over the overridden methods, so developers won’t lose any context. For any extra helper functions unique to a given handler, we can certainly document those in place. What do you think? Does that approach sound reasonable to you? |
Agree, I didn't know they share the same exact methods! I'll relaunch, thank you for your comments here. |
Documentation added for the
model_handler
part of the BFCL benchmark