-
Notifications
You must be signed in to change notification settings - Fork 481
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
AI Tutor - set up how first 3 prototypes interact #55015
Conversation
const handleSend = async studentCode => { | ||
dispatch( | ||
askAITutor({systemPrompt: systemPrompt, studentCode: studentCode}) | ||
); |
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.
- 🤔 openAi also needs access to the test file(s)... find out where those live and how to send them along in the request
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.
Are you planning to add that within the scope of this PR?
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.
No - this was a note to myself. I'll disable the button for now and build out sending the test files along with the request in the next PR. Thanks for checking!
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.
That work is happening here: #55076
@@ -14,24 +14,26 @@ const initialState: AITutorState = { | |||
isWaitingForAIResponse: false, | |||
}; | |||
|
|||
interface question { |
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.
nit: I think interface
s are meant to be capitalized?
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 also wonder if a broader name for this is better, like ChatContext
. (I think of "question" and "userMessage" -- a term used in Aichat for the next message the user sends to the endpoint -- as being equivalent.)
}; | ||
const AITutorPanel = props => { | ||
const dispatch = useDispatch(); | ||
const {level} = props; |
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.
nit: You could destructure this in props, i.e. const AITutorPanel = ({level}) => { ... })
.
<img alt={i18n.aiBot()} src={icon} className={style.aiBotImg} /> | ||
{isCodingLevel && <CompilationAssistant levelId={level.id} />} | ||
</AITutorPanelContainer> | ||
const handleCheckboxChange = key => { |
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 only one can be checked, could you store the key associated with the checked box instead of the whole array of checkbox objects and their properties on state?
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.
🤔 Every time a box is checked, I need to uncheck any previously checked boxes so I think I'd need state for the box to check as well as the ones to uncheck. It seemed simpler in my mind to just store all of them in state. What am I missing?
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 struggled to enroll in the experiment locally so I couldn't test this, but here's the minor simplification I think would also work: https://github.com/code-dot-org/code-dot-org/pull/55082/files.
Though, to Molly's point, it looks like there's a RadioButton
component available, which might better represent the intended mutual exclusivity: https://github.com/code-dot-org/code-dot-org/blob/staging/apps/src/componentLibrary/radioButton/README.md.
|
||
// AI Tutor feature that explains to students why their code did not compile. | ||
const CompilationTutor = props => { | ||
const levelId = props.levelId; |
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.
Same nit as above.
)} | ||
{javalabState.hasRunOrTestedCode && | ||
!javalabEditorState.hasCompilationError && ( | ||
<h4>🎉 Your code is compiling successfully. Great work!</h4> |
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.
Was it a conscious choice not to start a translation file for these?
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.
yes, it seems a little early since they aren't even really placeholders for string we know we'll need just part of my rogue self-organization scheme 😆 I'm not even sure that CSA is translated. But it's a good reminder for the kinds of work we'll need to leave time for as we near a more finalized notion of what AI Tutor will be and what strings will be involved.
|
||
// AI Tutor feature that explains to students why their code is not passing tests. | ||
const ValidationTutor = props => { | ||
const levelId = props.levelId; |
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.
Same nit, last time 😄
A few small comments, but nice work on getting a framework for all of these set up! 🎉 |
{checkboxes.map( | ||
cb => | ||
!cb.hidden && ( | ||
<Checkbox |
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 we do radio buttons instead of checkboxes that will enforce that only once can be selected right?
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.
@molly-moen @ebeastlake Updated to Radio Buttons!
Screen.Recording.2023-11-21.at.11.48.34.AM.mov
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.
Looks good, one redux suggestion!
// AI Tutor feature that explains to students why their code did not compile. | ||
const CompilationTutor = ({levelId}) => { | ||
const dispatch = useDispatch(); | ||
const javalabState = useSelector(state => state.javalab); |
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.
Instead of getting all the state here I think we should get individual pieces as separate constants (for example, const hasRunOrTestedCode = useSelector(state => state.javalab.hasRunOrTestedCode);
. This will prevent extra re-renders when something changes in the java lab state that we don't care about in this component.
I like Molly's suggestion, but my feedback has been addressed! Nice work, Erin! |
The goal is to start 2024 with 3 working prototypes for AI Tutor: 1.) Code Compilation 2.) Validation and 3.) General Chat.
This early work sets up a possible mechanism for how those prototypes can co-exist in the same space. UI is NOT final! Currently no one externally can see any of the AI Tutor panel content. We just need a place to iterate and inch toward resolving some of the ambiguity of what AI Tutor might eventually evolve into.
In the AI Tutor panel, I added checkboxes so a student can indicate what kind of help they'd like to get from the AI Tutor. It's a single select - only one option is possible at a time.
On non-coding CSA levels, the only option is the generic "I have a question" which when selected just shows "Coming soon!"
On non-validated coding levels, there are options for "Code compilation" and "I have a question":
If "Code compilation" is selected, but the code has not yet been run, the student is prompted to run their code:
If "Code compilation" is selected, the code has been run, and it compiles successfully, we celebrate:
If "Code compilation" is selected, the code has been run, and there are errors, the enabled "Ask AI Tutor" button is available:
On validated coding levels, there are options for "Code compilation", "Falling tests" and "I have a question":
If "Failing tests" is selected, and the tests have not yet been run, the student is prompted to run tests:
If "Failing tests" is selected, and the tests have run but there is a compilation error, the student is prompted to fix the compilation errors first (they can toggle to "Code compilation" and get the enabled "Ask AI Tutor" button"):
If "Failing tests" is selected, and the tests have run without compilation errors, but fail, they will see the "Ask AI Tutor" button. Note this button is currently always disabled because it doesn't yet return a meaningful response because I still need to pass the test files along with the student code to openAI. The AI just gets confused and times out right now.
If "Failing tests" is selected, and the tests have run without compilation errors, and succeed, celebrate:
Links
Testing story
Deployment strategy
Follow-up work
Privacy
Security
Caching
PR Checklist: