-
Notifications
You must be signed in to change notification settings - Fork 788
feat: add GetCellOutputs tool and get_llm_context for all UIElement classes #6889
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: add GetCellOutputs tool and get_llm_context for all UIElement classes #6889
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
for more information, see https://pre-commit.ci
| """ | ||
| return False | ||
|
|
||
| def _get_llm_context(self) -> str: |
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.
could we call this _repr_ai_?
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.
Yup!
|
|
||
| md_context = f""" | ||
| ### UI Element: `{component_name}` | ||
| Label: \"{label}\" |
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.
i would assume the LLM can see the code. im not sure how valuable it is to pass all this information.
is there a more concise version that just consist of info it cannot see. e.g
repr_args = {arg_key: repr(arg_value)[:100] for arg_key, arg_value in args}
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.
Agree with this, actually I think the info it cannot see is just the defaults, which already appear in the signature. Maybe we can omit args completely?
marimo/_messaging/ops.py
Outdated
| name: str | ||
| value: Optional[str] | ||
| datatype: Optional[str] | ||
| llm_context: Optional[str] = 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.
this is going to add additional computation cost and network costs passing this over the wire, especially when it is rarely used (variables are sent over a lot).
my prefernce would be to split this feature out from the GetCellOuputsTool while we experimrent with adding this context to each variable.
ideally this can be an addon and not need to be placed in the common path.
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.
Got it! So we can't put llm_context in VariableValue. If ever it will need to be accessed some other way, ideally directly from the UIElement via repr_ai.
|
I removed llm_context and ui_element handling for now. It will still be included in the visual output as HTML. We'll iterate on this in future PRs @mscolnick |
marimo/_runtime/runtime.py
Outdated
| } | ||
| referring_cells.update( | ||
| self.update_stateful_values(bound_names, value) | ||
| self.update_stateful_values( |
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.
If these are now just whitespace changes, can we checkout this file?
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.
On it. I'm also adding tests for the new tool
📝 Summary
This adds GetCellOuputs for visual outputs, console outputs, and marimo UIElement context with configuration and signature of element. This also adds llm_context to all tools that return
VariableValue.Sample outputs:
Markdown Cell Output:
{ "visual_output": "<h1>Sales Analysis</h1>", "visual_mimetype": "text/markdown", "stdout": [], "stderr": [], }Console Cell Output:
{ "visual_output": "", "visual_mimetype": "text/plain", "stdout": ["Hello you!!\n"], "stderr": [], }Chart Cell Output:
{ "visual_output": "{ Full Vega-Lite JSON spec with 9 data points }", "visual_mimetype": "application/vnd.vegalite.v5+json", "stdout": [], "stderr": [], }🔍 Description of Changes
GetCellOutputstoolGetCellOutputstool📋 Checklist