-
Notifications
You must be signed in to change notification settings - Fork 285
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
feat(playground): add tools ui #5002
Conversation
2d394a3
to
0c71f2d
Compare
linter(jsonParseLinter()), | ||
linter(jsonSchemaLinter(), { needsRefresh: handleRefresh }), | ||
jsonLanguage.data.of({ | ||
autocomplete: jsonCompletion(), |
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.
Very cool
"theme" | "extensions" | "editable" | ||
>; | ||
|
||
export function JSONToolEditor(props: JSONToolEditorProps) { |
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.
Isn't this really JSONEditor with json schema added on top?
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.
Yeah i had baked in the tool schema, so that's what makes it "tool" I guess but i could either
- modify the json editor to accept a schema and conditionally add the extensions or
- make a new jsoneditorwithschema (still would need to take in a schema as a prop)
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.
removed and extended the jsonEditor
e8ee023
to
55a6024
Compare
<div | ||
key={i} | ||
key={instance.id} | ||
css={css` | ||
flex: 1 1 0px; |
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.
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.
ahh yes thank you i saw this then forgot to circle back
82cab7a
to
bedd86b
Compare
jsonSchemaLinter, | ||
stateExtensions, | ||
} from "codemirror-json-schema"; | ||
import { JSONSchema7 } from "json-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.
what's the 7 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.
the version of the json schema spec i believe
const toolNames = useMemo( | ||
() => | ||
tools | ||
.map((tool) => tool.definition.function?.name) | ||
.filter((name): name is NonNullable<typeof name> => name != null), | ||
[tools] | ||
); |
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.
note there's a slight bug here if the names collide
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.
Very cool
resolves #4902
Screen.Recording.2024-10-15.at.10.53.34.AM.mov
with choice selector