Send preloaded CODAP document info to LLM (PT-188713175) #24
Conversation
lublagg
force-pushed
the
188713175-provide-codap-context
branch
from
035d9a9 to
94a7fc2
Compare
| try { | ||
| self.transcriptStore.addMessage(DEBUG_SPEAKER, {description: "Sending CODAP document information to LLM", content: message}); | ||
| const messageSent = yield self.apiConnection.beta.threads.messages.create(self.thread.id, { | ||
| role: "user", |
There was a problem hiding this comment.
Would it make sense to change this role value to "system"?
There was a problem hiding this comment.
Unfortunately with the Assistants API they don't allow that role value, only "user" and "assistant."
There was a problem hiding this comment.
Mostly working great - I can see the information sent at initialization, and updates sent if I change data in the table. If I ask about the columns in the data, no requests are sent to CODAP and I get a faster response compared to if I ask about the rows, which reqires additional info to be requested from CODAP.
As discussed in Slack:
- new messages triggered by the handlers result in errors if there is another
runactive. - newly-created data contexts need to have change handlers set up.
src/components/App.tsx
Outdated
| await initializePlugin({pluginName: kPluginName, version: kVersion, dimensions}); | ||
| addDataContextsListListener(handleDocumentChangeNotice); | ||
| const dataContexts = await getListOfDataContexts(); | ||
| dataContexts.values.forEach((ctx: Record<string, any>) => addDataContextChangeListener(ctx.name, handleDataContextChangeNotice)); |
There was a problem hiding this comment.
This adds a dataContextChangeListener for each context that exists at startup. Do you also need to add one to contexts that are added later, when you're handling the documentchange?
|
|
||
| const sendCODAPDocumentInfo = flow(function* (message) { | ||
| try { | ||
| self.transcriptStore.addMessage(DEBUG_SPEAKER, {description: "Sending CODAP document information to LLM", content: message}); |
There was a problem hiding this comment.
Should this be using formatJsonMessage(message) instead of just message?
|
|
||
| // we don't send case-level information, only info about dataContexts, collections + attributes | ||
| // documentation about CODAP Data Interactive API notifications: https://github.com/concord-consortium/codap/wiki/CODAP-Data-Interactive-Plugin-API#codap-initiated-actions | ||
| export const notificationsToIgnore = [ |
There was a problem hiding this comment.
Would it make sense to put this in src/constants.ts?
This PR makes the following changes: