Fix: Always display objective scores; auxiliary scores shown only if requested (Issue #1096) #1101
Conversation
|
Thanks for addressing this! |
Apply reviewer suggestion: use list comprehension for objective score Co-authored-by: hannahwestra25 <[email protected]>
| if scores: | ||
| print() | ||
| self._print_colored(f"{self._indent}📊 Scores:", Style.DIM, Fore.MAGENTA) | ||
| objective_score = [score for score in scores if score.score_category == "objective"][0] |
There was a problem hiding this comment.
That is not actually what the category is for/does.
After reviewing the code, I don't believe there is a way to do this right now unless we add some kind of metadata for the objective scorer to identify the scoring records and set them apart from other scoring records.
@rlundeen2 @bashirpartovi thoughts?
There was a problem hiding this comment.
Thanks for the clarification, @romanlutz that makes sense. I used score_category == "objective" here since it was the only discriminator readily available in the current code, but I understand that’s not really what the field is intended for.
I’m happy to adjust this if the team prefers adding explicit metadata to distinguish the objective scorer from the other scoring records. Let me know if you’d like me to make that change in this PR, or if it would be better to wait until we decide on the right long-term design for identifying objective scores.
There was a problem hiding this comment.
Checked with @rlundeen2 who had the correct observation that AttackResult has the last_score. That is only the objective score of the last iteration. We would still miss objective scores from earlier iterations unless we compare that scorer_class_identifier with the ones from previous iterations and if it's a match we'll present it as objective score. It's a bit tedious but definitely the easiest way to get it done without changes to other parts of the code.
There was a problem hiding this comment.
Actually nevermind, just display the last_score from AttackResult when it shows up for one of the responses. We don't need the objective scores for every iteration. If someone wants all scores they can set the flag to print all aux scores.
There was a problem hiding this comment.
Thanks for clarifying, @romanlutz. That makes sense. I’ll adjust the implementation to display only the last_score from AttackResult as suggested. This keeps the change straightforward while still ensuring the objective score is shown. If users need full iteration details, they can indeed enable the auxiliary scores flag.
Update
This PR fixes Issue #1096:
Related Issue
Fixes #1096