Skip to content

Added multimodal logic to aichat_request_chat_completion_job #65328

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

Open
wants to merge 3 commits into
base: staging
Choose a base branch
from

Conversation

tshaffercodeorg
Copy link
Contributor

@tshaffercodeorg tshaffercodeorg commented Apr 17, 2025

Change Summary

  • Added in multimodal flagging function (.multimodal?)
  • Added in flag and counts to .report_job_start and .report_job_finish
  • Updated tests to match new dimension lengths

New Metrics Payload in Cloudwatch

image

@tshaffercodeorg tshaffercodeorg marked this pull request as ready for review April 18, 2025 20:21
@tshaffercodeorg tshaffercodeorg self-assigned this Apr 18, 2025
@tshaffercodeorg tshaffercodeorg requested a review from a team April 18, 2025 20:21
Comment on lines +55 to +58
messages_with_assets_count = messages.count do |message|
message['assets']&.any?
end
return messages_with_assets_count>0
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: you can simplify the return value to just

messages.count do |message|
  message['assets']&.any?
end

rather than storing a message_with_assets_count local variable

@@ -50,6 +50,14 @@ def perform(request:, locale:)
request.update!(response: response, execution_status: status)
end

private def multimodal?(request)
Copy link
Contributor

Choose a reason for hiding this comment

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

Minor: can we move this below around line 110 so it's grouped with the other similar get_model_id helper?

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.

2 participants