Skip to content
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

Add chat-plugin #134

Open
wants to merge 66 commits into
base: main
Choose a base branch
from
Open

Add chat-plugin #134

wants to merge 66 commits into from

Conversation

vzhang03
Copy link
Contributor

Wanted to submit this PR to get a review on what changes should be worked on next. The functionality is all working as intended and has gone through testing by fellow URSI team. This plugin contains functionality to customize a chat with an LLM supporting various strategies for controlling bot-prompting and user prompting. This plugin also collects user keystrokes and comprehensive data on the conversation and communication with the LLM.

Main thing to work on is documentation and specifically how to guide people to navigate the process of using the backend. This plugin is hard to work with for people without coding experience, but not sure how to remedy this issue.

A smaller issue is a code rewrite moving the fetching and calling into a separate class. This would make it more friendly for devs to work with. Haven't done because prioritized building out prompting features for the other group. Not sure if necessary or if should just comment on the associated methods.

Lastly, should figure out where to handle styling and if should move away from styles.css to another place as well as allow researcher to customize.

Victor Zhang and others added 30 commits June 3, 2024 09:35
… that triggers after a certain number of messages or response time prompting the user and creating a continue button
vzhang03 added 27 commits June 28, 2024 12:21
…and fixing data collection to be more legible
Copy link

changeset-bot bot commented Aug 16, 2024

🦋 Changeset detected

Latest commit: 80b5c60

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@jspsych-contrib/plugin-chat Major

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@jodeleeuw
Copy link
Member

Hey @vzhang03 and @Bankminer78, any interest in updating this for version 8? Have a student project that wants to use it. I can work on it soon if neither of you has the time right now 😁

Copy link
Collaborator

@jadeddelta jadeddelta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

did as much as i could without having an openai key to test on, but here are some general suggestions and potential fixes to pursue with parameterization of this plugin

@@ -41,6 +41,13 @@ const info = <const>{
type: ParameterType.STRING,
default: null,
},
/**
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

artifact from a previous commit?


| Parameter | Type | Default Value | Description |
| ------------------- | ---------------- | ------------------ | ---------------------------------------- |
| ai_prompt | String | undefined | System prompt that will be fed to GPT in order to prompt the chatbot. This is the most simple prompting parameter and should be used in cases where you want to |
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not finished description

| ai_prompt | String | undefined | System prompt that will be fed to GPT in order to prompt the chatbot. This is the most simple prompting parameter and should be used in cases where you want to |
| ai_model | String | gpt-4o-mini | Refers to the OpenAI model that will be called during the chats. |
| continue_button | Object | { message_trigger: 0 } | Allows you to customize when the continue_button appears. The three options that can be passed in the object are "timer_trigger", "message_trigger", and "message". "Timer_trigger" referes to the milliseconds from initialization, "message_trigger" refers to the total number of messages sent by the user and "message" is the message to be displayed when the button appears. This is an optional parameter that should be used to customize the length of conversation. |
| ai_model | String | gpt-4o-mini | OpenAI model that will be called during the chats. |
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

duplication?


jest.useFakeTimers();

describe("my plugin", () => {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

guessing that it would be really hard to mock this sort of plugin, so best to just remove the test and update the package.json respectively

const info = <const>{
name: "chat",
parameters: {
// BOOL, STRING, INT, FLOAT, FUNCTION, KEY, KEYS, SELECT, HTML_STRING, IMAGE, AUDIO, VIDEO, OBJECT, COMPLEX
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

best to TSDoc these as well for developer ease of use

// type: ParameterType.STRING,
// default: undefined,
// },
continue_button: {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it worth grouping all of these? or can we just have three separate parameters?

array: true,
default: [],
},
selection_prompt: {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

selection_prompt: { selection_prompt: ... } feels a bit unwieldy, maybe selection_stimulus?

},
message_trigger: {
type: ParameterType.INT,
default: 99999999999999999999999, // silencing error message
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

possible to -1? not sure why this would need one when the others don't especially if prompt_chain's default is {}

trial.selection_prompt["message_trigger"] === null &&
trial.selection_prompt["timer_trigger"] === null
) {
console.error("Missing required trigger property in selection_prompt, will never trigger");
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style, but best to use throw new Error("....") to halt execution


const system_user =
this.selection_prompt["selection_prompt"] +
"Task: You should select one of the three possible responses that would best achieve your goal with the intention of outputting it to the user" +
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

possible to parameterize this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants