-
Notifications
You must be signed in to change notification settings - Fork 3
Add json schema #7
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
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
Rolland-He
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.
Hi, @wkukka1 nice work! I’ve left a inline comment regarding magic strings, mostly suggesting to extract repeated literals into named constants. Just a minor style suggestion and totally fine to leave as is. Everything else looks good!
ai_server/server.py
Outdated
| messages = _build_messages(content, system_prompt, image_files=[]) # TODO: Pass image files | ||
| payload = {'model': model, 'messages': messages, 'stream': False, 'max_tokens': 512} | ||
| if json_schema: | ||
| payload['json_schema'] = json_schema["schema"] |
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.
For coding style, try to define "schema" as a constant like SCHEMA_KEY = "schema" and reuse it throughout. This makes the intent clearer and avoids potential typos. Similarly for "--json-schema".
ai_server/server.py
Outdated
| model=model, | ||
| messages=messages, | ||
| stream=False, | ||
| format=json_schema['schema'] if json_schema else None, |
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.
Same here.
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.
It's still 'schema'.
| cmd = [LLAMA_CPP_CLI, '-m', model_path, '--n-gpu-layers', '40', '-p', content, '-n', '512', '--single-turn'] | ||
| if json_schema: | ||
| raw_schema = json_schema["schema"] if "schema" in json_schema else json_schema | ||
| cmd += ["--json-schema", json.dumps(raw_schema)] |
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.
Same here.
for more information, see https://pre-commit.ci
Rolland-He
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.
Hi @wkukka1 , good work! There's still one place left.
ai_server/server.py
Outdated
| model=model, | ||
| messages=messages, | ||
| stream=False, | ||
| format=json_schema['schema'] if json_schema else None, |
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.
It's still 'schema'.
Rolland-He
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.
Looks good!
test/test_server_mode.py
Outdated
| assert args[0] == "http://localhost:8080/v1/chat/completions" | ||
|
|
||
| body = kwargs["json"] | ||
| print(body) |
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.
Delete this line
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!
Updates:
json_schemaparam in post request to/chatendpoint