-
Notifications
You must be signed in to change notification settings - Fork 53
AI-powered marking #1248
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
base: master
Are you sure you want to change the base?
AI-powered marking #1248
Changes from all commits
5cd6ac6
853ba84
4c37d14
8192b3d
8a235b3
d384e06
98feac2
7716d57
df34dbd
0a25fa8
2723f5a
02f7ed1
cb34984
09a7b09
ed44a7e
3715368
5bfe276
c27b93b
17884fd
f91cc92
81e5bf7
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 |
---|---|---|
|
@@ -118,3 +118,6 @@ erl_crash.dump | |
|
||
# Generated lexer | ||
/src/source_lexer.erl | ||
|
||
# Ignore log files | ||
/log |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,95 @@ | ||
defmodule Cadet.AIComments do | ||
@moduledoc """ | ||
Handles operations related to AI comments, including creation, updates, and retrieval. | ||
""" | ||
|
||
import Ecto.Query | ||
alias Cadet.Repo | ||
alias Cadet.AIComments.AIComment | ||
|
||
@doc """ | ||
Creates a new AI comment log entry. | ||
""" | ||
def create_ai_comment(attrs \\ %{}) do | ||
%AIComment{} | ||
|> AIComment.changeset(attrs) | ||
|> Repo.insert() | ||
end | ||
|
||
@doc """ | ||
Gets an AI comment by ID. | ||
""" | ||
def get_ai_comment!(id), do: Repo.get!(AIComment, id) | ||
|
||
@doc """ | ||
Retrieves an AI comment for a specific submission and question. | ||
Returns `nil` if no comment exists. | ||
""" | ||
def get_ai_comments_for_submission(submission_id, question_id) do | ||
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. Naming implies you are getting all AI comments. Also what is the use case for getting only one of the comments? |
||
Repo.one( | ||
from(c in AIComment, | ||
where: c.submission_id == ^submission_id and c.question_id == ^question_id | ||
) | ||
) | ||
end | ||
|
||
@doc """ | ||
Retrieves the latest AI comment for a specific submission and question. | ||
Returns `nil` if no comment exists. | ||
""" | ||
def get_latest_ai_comment(submission_id, question_id) do | ||
Repo.one( | ||
from(c in AIComment, | ||
where: c.submission_id == ^submission_id and c.question_id == ^question_id, | ||
order_by: [desc: c.inserted_at], | ||
limit: 1 | ||
) | ||
) | ||
end | ||
|
||
@doc """ | ||
Updates the final comment for a specific submission and question. | ||
Returns the most recent comment entry for that submission/question. | ||
""" | ||
def update_final_comment(submission_id, question_id, final_comment) do | ||
comment = get_latest_ai_comment(submission_id, question_id) | ||
|
||
case comment do | ||
nil -> | ||
{:error, :not_found} | ||
|
||
_ -> | ||
comment | ||
|> AIComment.changeset(%{final_comment: final_comment}) | ||
|> Repo.update() | ||
end | ||
end | ||
|
||
@doc """ | ||
Updates an existing AI comment with new attributes. | ||
""" | ||
def update_ai_comment(id, attrs) do | ||
id | ||
|> get_ai_comment!() | ||
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. This could raise an error which isn't handled (id not found in DB) |
||
|> AIComment.changeset(attrs) | ||
|> Repo.update() | ||
end | ||
|
||
@doc """ | ||
Updates the chosen comments for a specific submission and question. | ||
Accepts an array of comments and replaces the existing array in the database. | ||
""" | ||
def update_chosen_comments(submission_id, question_id, new_comments) do | ||
comment = get_latest_ai_comment(submission_id, question_id) | ||
|
||
case comment do | ||
nil -> | ||
{:error, :not_found} | ||
|
||
_ -> | ||
comment | ||
|> AIComment.changeset(%{comment_chosen: new_comments}) | ||
|> Repo.update() | ||
end | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,36 @@ | ||
defmodule Cadet.AIComments.AIComment do | ||
@moduledoc """ | ||
Defines the schema and changeset for AI comments. | ||
""" | ||
|
||
use Ecto.Schema | ||
import Ecto.Changeset | ||
|
||
schema "ai_comment_logs" do | ||
field(:submission_id, :integer) | ||
field(:question_id, :integer) | ||
Comment on lines
+10
to
+11
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. If these are FKs, you need to specify that in the schema |
||
field(:raw_prompt, :string) | ||
field(:answers_json, :string) | ||
field(:response, :string) | ||
field(:error, :string) | ||
field(:comment_chosen, {:array, :string}) | ||
field(:final_comment, :string) | ||
|
||
timestamps() | ||
end | ||
|
||
def changeset(ai_comment, attrs) do | ||
ai_comment | ||
|> cast(attrs, [ | ||
:submission_id, | ||
:question_id, | ||
:raw_prompt, | ||
:answers_json, | ||
:response, | ||
:error, | ||
:comment_chosen, | ||
:final_comment | ||
]) | ||
|> validate_required([:submission_id, :question_id, :raw_prompt, :answers_json]) | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -941,7 +941,7 @@ | |
raw_answer, | ||
force_submit | ||
) do | ||
with {:ok, team} <- find_team(question.assessment.id, cr_id), | ||
{:ok, submission} <- find_or_create_submission(cr, question.assessment), | ||
{:status, true} <- {:status, force_submit or submission.status != :submitted}, | ||
{:ok, _answer} <- insert_or_update_answer(submission, question, raw_answer, cr_id) do | ||
|
@@ -2289,8 +2289,8 @@ | |
@spec get_answers_in_submission(integer() | String.t()) :: | ||
{:ok, {[Answer.t()], Assessment.t()}} | ||
| {:error, {:bad_request, String.t()}} | ||
def get_answers_in_submission(id) when is_ecto_id(id) do | ||
answer_query = | ||
def get_answers_in_submission(id, question_id \\ nil) when is_ecto_id(id) do | ||
base_query = | ||
Answer | ||
|> where(submission_id: ^id) | ||
|> join(:inner, [a], q in assoc(a, :question)) | ||
|
@@ -2312,6 +2312,13 @@ | |
{s, student: {st, user: u}, team: {t, team_members: {tm, student: {tms, user: tmu}}}} | ||
) | ||
|
||
answer_query = | ||
if is_nil(question_id) do | ||
base_query | ||
else | ||
base_query |> where(question_id: ^question_id) | ||
end | ||
|
||
Comment on lines
+2315
to
+2321
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. Use |
||
answers = | ||
answer_query | ||
|> Repo.all() | ||
|
@@ -2692,7 +2699,7 @@ | |
|
||
def has_last_modified_answer?( | ||
question = %Question{}, | ||
cr = %CourseRegistration{id: cr_id}, | ||
last_modified_at, | ||
force_submit | ||
) do | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -14,6 +14,8 @@ defmodule Cadet.Courses.Course do | |
enable_achievements: boolean(), | ||
enable_sourcecast: boolean(), | ||
enable_stories: boolean(), | ||
enable_llm_grading: boolean(), | ||
llm_api_key: String.t() | nil, | ||
source_chapter: integer(), | ||
source_variant: String.t(), | ||
module_help_text: String.t(), | ||
|
@@ -28,6 +30,8 @@ defmodule Cadet.Courses.Course do | |
field(:enable_achievements, :boolean, default: true) | ||
field(:enable_sourcecast, :boolean, default: true) | ||
field(:enable_stories, :boolean, default: false) | ||
field(:enable_llm_grading, :boolean) | ||
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. Should this be defaulted to false |
||
field(:llm_api_key, :string) | ||
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. Am I understanding this right that you are going to have separate API keys for each course...? Why is it not just set in the env file. |
||
field(:source_chapter, :integer) | ||
field(:source_variant, :string) | ||
field(:module_help_text, :string) | ||
|
@@ -42,13 +46,59 @@ defmodule Cadet.Courses.Course do | |
|
||
@required_fields ~w(course_name viewable enable_game | ||
enable_achievements enable_sourcecast enable_stories source_chapter source_variant)a | ||
@optional_fields ~w(course_short_name module_help_text)a | ||
|
||
@optional_fields ~w(course_short_name module_help_text enable_llm_grading llm_api_key)a | ||
|
||
@spec changeset( | ||
{map(), map()} | ||
| %{ | ||
:__struct__ => atom() | %{:__changeset__ => map(), optional(any()) => any()}, | ||
optional(atom()) => any() | ||
}, | ||
%{optional(:__struct__) => none(), optional(atom() | binary()) => any()} | ||
) :: Ecto.Changeset.t() | ||
def changeset(course, params) do | ||
course | ||
|> cast(params, @required_fields ++ @optional_fields) | ||
|> validate_required(@required_fields) | ||
|> validate_sublanguage_combination(params) | ||
|> put_encrypted_llm_api_key() | ||
end | ||
|
||
def put_encrypted_llm_api_key(changeset) do | ||
if llm_api_key = get_change(changeset, :llm_api_key) do | ||
if is_binary(llm_api_key) and llm_api_key != "" do | ||
secret = Application.get_env(:openai, :encryption_key) | ||
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. Consider adding this to configs if this is something new. i.e. a comment for it in configs |
||
|
||
if is_binary(secret) and byte_size(secret) >= 16 do | ||
# Use first 16 bytes for AES-128, 24 for AES-192, or 32 for AES-256 | ||
key = binary_part(secret, 0, min(32, byte_size(secret))) | ||
# Use AES in GCM mode for encryption | ||
iv = :crypto.strong_rand_bytes(16) | ||
|
||
{ciphertext, tag} = | ||
:crypto.crypto_one_time_aead( | ||
:aes_gcm, | ||
key, | ||
iv, | ||
llm_api_key, | ||
"", | ||
true | ||
) | ||
|
||
# Store both the IV, ciphertext and tag | ||
encrypted = iv <> tag <> ciphertext | ||
put_change(changeset, :llm_api_key, Base.encode64(encrypted)) | ||
else | ||
add_error(changeset, :llm_api_key, "encryption key not configured properly") | ||
end | ||
else | ||
# If empty string or nil is provided, don't encrypt but don't add error | ||
changeset | ||
end | ||
else | ||
# The key is not being changed, so we need to preserve the existing value | ||
put_change(changeset, :llm_api_key, changeset.data.llm_api_key) | ||
end | ||
end | ||
|
||
# Validates combination of Source chapter and variant | ||
|
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.
Is there a reason why we use
get!
instead ofget