-
Notifications
You must be signed in to change notification settings - Fork 0
feat: link data to graph image for description (DAVAI-38) #46
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
Changes from 3 commits
f504c56
5feb83c
e9bf5b9
567c18a
3dd1820
df9009c
7fa833a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -26,6 +26,14 @@ const OpenAIType = types.custom({ | |
}, | ||
}); | ||
|
||
interface IGraphAttrData { | ||
legend?: Record<string, any>; | ||
rightSplit?: Record<string, any>; | ||
topSplit?: Record<string, any>; | ||
xAxis?: Record<string, any>; | ||
yAxis?: Record<string, any>; | ||
} | ||
|
||
/** | ||
* AssistantModel encapsulates the AI assistant and its interactions with the user. | ||
* It includes properties and methods for configuring the assistant, handling chat interactions, and maintaining the assistant's | ||
|
@@ -62,9 +70,10 @@ export const AssistantModel = types | |
transcriptStore: ChatTranscriptModel | ||
}) | ||
.volatile(() => ({ | ||
updateSonificationStoreAfterRun: false, | ||
dataUri: "", | ||
dataContextForGraph: null as IGraphAttrData | null, | ||
uploadFileAfterRun: false, | ||
dataUri: "" | ||
updateSonificationStoreAfterRun: false, | ||
})) | ||
.actions((self) => ({ | ||
addDavaiMsg(msg: string) { | ||
|
@@ -133,7 +142,6 @@ export const AssistantModel = types | |
} | ||
}); | ||
|
||
|
||
const sendDataCtxChangeInfo = flow(function* (msg: string) { | ||
try { | ||
if (self.isLoadingResponse || self.isCancelling || self.isResetting) { | ||
|
@@ -240,6 +248,7 @@ export const AssistantModel = types | |
yield sendFileMessage(fileId); | ||
self.uploadFileAfterRun = false; | ||
self.dataUri = ""; | ||
self.dataContextForGraph = null; | ||
startRun(); | ||
} else { | ||
const messages = yield self.apiConnection.beta.threads.messages.list(self.thread.id); | ||
|
@@ -283,6 +292,67 @@ export const AssistantModel = types | |
} | ||
}); | ||
|
||
const getAttributeData = flow(function* (graphID: string, attrID: string | null) { | ||
if (!attrID) return { attributeData: null }; | ||
|
||
const response = yield Promise.resolve(codapInterface.sendRequest({ | ||
action: "get", | ||
resource: `component[${graphID}].attribute[${attrID}]` | ||
})); | ||
|
||
const attributeData = response?.values | ||
? { | ||
id: response.values.id, | ||
name: response.values.name, | ||
values: response.values._categoryMap.__order | ||
} | ||
: null; | ||
|
||
return { attributeData }; | ||
}); | ||
|
||
const getGraphAttrData = flow(function* (graphID) { | ||
try { | ||
const graph = yield getGraphByID(graphID); | ||
if (graph) { | ||
const legendAttrData = yield getAttributeData(graphID, graph.legendAttributeID); | ||
const rightAttrData = yield getAttributeData(graphID, graph.rightSplitAttributeID); | ||
const topAttrData = yield getAttributeData(graphID, graph.topSplitAttributeID); | ||
const xAttrData = yield getAttributeData(graphID, graph.xAttributeID); | ||
const yAttrData = yield getAttributeData(graphID, graph.yAttributeID); | ||
const y2AttrData = yield getAttributeData(graphID, graph.yAttributeIDs ? graph.yAttributeIDs[1] : null); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are you sure that There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Possibly you could use a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @bgoldowsky Thanks for asking. In looking into this more, it seems what we actually want to do is |
||
|
||
// Combine y-axis data if we have a second y-axis | ||
const combinedYAxisData = y2AttrData.attributeData | ||
? { attributeData: [yAttrData.attributeData, y2AttrData.attributeData] } | ||
: yAttrData; | ||
|
||
const graphAttrData = { | ||
legend: legendAttrData, | ||
rightSplit: rightAttrData, | ||
topSplit: topAttrData, | ||
xAxis: xAttrData, | ||
yAxis: combinedYAxisData | ||
}; | ||
|
||
if (graphAttrData) { | ||
self.addDbgMsg("Data context for graph", formatJsonMessage(graphAttrData)); | ||
return graphAttrData; | ||
} else { | ||
self.addDbgMsg("No data context found for graph", formatJsonMessage(graph)); | ||
emcelroy marked this conversation as resolved.
Show resolved
Hide resolved
|
||
return null; | ||
} | ||
} else { | ||
self.addDbgMsg("No graph found with ID", graphID); | ||
return null; | ||
} | ||
} catch (err) { | ||
console.error("Failed to get graph attribute data:", err); | ||
self.addDbgMsg("Failed to get graph attribute data", formatJsonMessage(err)); | ||
return null; | ||
} | ||
}); | ||
|
||
const sendFileMessage = flow(function* (fileId) { | ||
try { | ||
const res = yield self.apiConnection.beta.threads.messages.create(self.thread.id, { | ||
|
@@ -297,6 +367,10 @@ export const AssistantModel = types | |
image_file: { | ||
file_id: fileId | ||
} | ||
}, | ||
{ | ||
type: "text", | ||
text: `The following JSON data describes key aspects of the graph in the image. Use this context to improve your interpretation and explanation of the graph. ${JSON.stringify(self.dataContextForGraph)}` | ||
} | ||
] | ||
}); | ||
|
@@ -336,6 +410,9 @@ export const AssistantModel = types | |
if (isImageSnapshotRequest) { | ||
self.uploadFileAfterRun = true; | ||
self.dataUri = res.values.exportDataUri; | ||
const graphID = resource.match(/\[(\d+)\]/)?.[1]; | ||
emcelroy marked this conversation as resolved.
Show resolved
Hide resolved
|
||
// We also send data for the attributes on the graph for additional context | ||
self.dataContextForGraph = yield getGraphAttrData(graphID); | ||
} | ||
// remove any exportDataUri value that exists since it can be large and we don't need to send it to the assistant | ||
res = isImageSnapshotRequest | ||
|
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 not clear why you want a one-element object here, rather than just returning
attributeData
directly.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.
@bgoldowsky It made the definition for
graphAttrData
ingetGraphAttrData
cleaner. I guess it is a bit odd, though. In a new commit, I changed it to just returnattributeData
, and then ingraphAttrData
, I added theattributeData
property name. I think it's kind of important to have that property name for each axis to make it absolutely clear to the LLM what the data is. There may be a way to do that more concisely, but I can't think of anything at the moment.