-
Notifications
You must be signed in to change notification settings - Fork 14.1k
Add basic support for function calls in oai python server #3431
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: master
Are you sure you want to change the base?
Conversation
when functions and function_call is specified in chat completion requests it generates and uses a grammar for the json scheme given in functions[function_call]
… and include sent function results in chat prompt
|
Why does this PR include commented-out code? |
|
Good question, removed it now. |
ejones
left a comment
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.
Love to see grammar getting implemented in the OAI layer! As for the JSON schema conversion, I might consider an approach like this:
- start with
examples/json-schema-to-grammar.py- I'm fairly confident in its handling of nesting, quoting, escaping, etc. - before processing, do a pass to inline any
$refsfrom$defs, and then just process as usual withjson-schema-to-grammar(or alternatively, add support for$refs/$defstojson-schema-to-grammar)
| prompt += f"{system_n}{line['content']}" | ||
| if (line["role"] == "user"): | ||
| prompt += f"{user_n}{line['content']}" | ||
| if (line["role"] == "function"): |
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.
nit: I assume this is to match the other conditionals but the parentheses are unnecessary
| if(is_present(body, "functions") and len(body["functions"])>0): | ||
| assert(is_present(body, "functions")) |
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.
nit: unnecessary outer parens in both statements
| postDataDecide = make_postData(body, chat=True, stream=False, function_call=function_call) | ||
| dataDecide = requests.request("POST", urllib.parse.urljoin(args.llama_api, "/completion"), data=json.dumps(postDataDecide)) |
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.
nit: idiomatic Python would be snake_case: post_data_decide etc
| grammar = "\n".join(rules) | ||
| grammar += ( # json base types | ||
| (r''' | ||
| ws ::= [ \t\n]? |
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 like ws is actually unused (which could be fine)?
| [^"\\] | | ||
| "\\" (["\\/bfnrt] | "u" [0-9a-fA-F] [0-9a-fA-F] [0-9a-fA-F] [0-9a-fA-F]) # escapes | ||
| )* "\"" | ||
| bool ::= "True" | "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.
Should be boolean and also lowercase:
boolean ::= "true" | "false"
| return f'"\\"" "{name}" "\\"" ":" {typename}' | ||
|
|
||
| properties = ' "," '.join([ | ||
| propery_to_grammar(name, schema_typename(prefix, prop, defs, arrs)) |
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.
Running make_grammar in isolation, I'm noticing nested objects get rendered as {func}-object rather than the specific rule name. I believe it comes down to the use of schema_typename here?
>>> print(make_grammar([{'name': 'foo', 'parameters': {'type': 'object', 'properties': {'a': {'type': 'object', 'properties': {'b': {'type': 'object', 'properties': {'c': {'type': 'number'}}}}}}}}], {'name': 'foo'}))
foo-b ::= "{" "\"" "c" "\"" ":" number "}"
foo-a ::= "{" "\"" "b" "\"" ":" foo-object "}"
foo ::= "{" "\"" "a" "\"" ":" foo-object "}"
| if etype == 'string': | ||
| return f'"\\"{repr(value)[1:-1]}\\""' | ||
| else: | ||
| return repr(value) |
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.
These still need to be quoted in the grammar, i.e., "1" | "2" instead of 1 | 2:
>>> print(make_grammar([{'name': 'foo', 'parameters': {'type': 'object', 'properties': {'a': {'type': 'number', 'enum': [1,2,3]}}}}], {'name': 'foo'}))
foo-a ::= ( 1 | 2 | 3 )
foo ::= "{" "\"" "a" "\"" ":" number "}"
| arrs[typename] = elemtype | ||
| return typename | ||
|
|
||
| def arr_to_rules(rules, prefix, name, elemtype): |
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.
Is there a reason arrays are treated specially, rather than a recursive case along with objects? Also, this approach seems to miss the element definition in the case of non-primitive elements:
>>> print(make_grammar([{'name': 'foo', 'parameters': {'type': 'object', 'properties': {'a': {'type': 'array', 'items': {'type': 'object', 'properties': {'a': {'type': 'number'}}}}}}}], {'name': 'foo'}))
foo ::= "{" "\"" "a" "\"" ":" foo-array-foo-object "}"
foo-array-foo-object ::= "[" ( foo-object ( "," foo-object )* )? "]"
root ::= foo
ws ::= [ \t\n]?
string ::=
"\"" (
[^"\\] |
"\\" (["\\/bfnrt] | "u" [0-9a-fA-F] [0-9a-fA-F] [0-9a-fA-F] [0-9a-fA-F]) # escapes
)* "\""
bool ::= "True" | "False"
integer ::= ("-"? ([0-9] | [1-9] [0-9]*))
number ::= ("-"? ([0-9] | [1-9] [0-9]*)) ("." [0-9]+)? ([eE] [-+]? [0-9]+)?
|
|
||
| def enum_to_rules(rules, prefix, name, schema): | ||
| enum_values = schema['enum'] | ||
| etype = schema['type'] |
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.
AFAIK, JSON Schema does not require type in conjunction with enum. The types could just be inferred from the values themselves. This allows the server to be more permissive in its handling of schemas.
|
|
||
| def decision_grammar(schema, root): | ||
| fnames = [fn['name'] for fn in schema] | ||
| fnames = [f'"\\"" "{fn}" "\\""' for fn in fnames] |
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.
In general, would need to escape fn here as it might contain quotes or escapes.
|
@ejones Ahh thanks for pointing out the |
|
Yeah I mean, given that it's an OAI compatibility layer, I don't think there's a place for the property order as a parameter anyway. FWIW, in |
tho while the json schema say it's unordered, the way the json converter works is by rearranging the unspecified properties in alphabetical order. for json schema that are generated they are generated in the order that property appears, so if it could have a default 'iteration order' over the schema properties instead of choosing alphabetical, you'd get a easier to live with default. |
I wanted to try magentic with the oai python server.
To work correctly the oai api for function calls needed to be implemented.
Function calls allow the LLM to respond with function calls instead of only text messages.
A grammar is automatically generated which constrains the LLM response by the provided function json schemes.
When the client replies with function results they will be included in the chat prompt.
Example usage with magentic: