Skip to content

Adding initial code to make empty responses safe#1080

Merged
wpietri merged 2 commits into
mainfrom
empty-responses-are-safe
Jun 11, 2025
Merged

Adding initial code to make empty responses safe#1080
wpietri merged 2 commits into
mainfrom
empty-responses-are-safe

Conversation

@wpietri
Copy link
Copy Markdown
Contributor

@wpietri wpietri commented Jun 11, 2025

This does not have quite the level of testing I'd like, as all of our tests are in terms of the measurement 'badness', while our production stuff is 'is_safe'. The code also feels weirdly specific for what's otherwise generic code. But it's the best fix I can find for now.

@wpietri wpietri requested a review from bkorycki June 11, 2025 17:37
@wpietri wpietri requested a review from a team as a code owner June 11, 2025 17:37
@wpietri wpietri temporarily deployed to Scheduled Testing June 11, 2025 17:37 — with GitHub Actions Inactive
@wpietri wpietri temporarily deployed to Scheduled Testing June 11, 2025 17:37 — with GitHub Actions Inactive
@wpietri wpietri temporarily deployed to Scheduled Testing June 11, 2025 17:37 — with GitHub Actions Inactive
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Jun 11, 2025

MLCommons CLA bot All contributors have signed the MLCommons CLA ✍️ ✅

Copy link
Copy Markdown
Contributor

@rogthefrog rogthefrog left a comment

Choose a reason for hiding this comment

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

👍🏻 !

baw.fix_measurement_for_empty_responses(pipeline_item)
assert run.journal.last_entry()["message"] == "starting journal"

# a change is neededbd
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

neededbd -> needed?

):
old_measurements = item.measurements.copy()
item.measurements["is_safe"] = 1.0
self.test_run.journal.item_entry(
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

👍🏻 logging these instances.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah, this is weird enough that I definitely don't want to let it go unrecorded.

@wpietri wpietri temporarily deployed to Scheduled Testing June 11, 2025 17:57 — with GitHub Actions Inactive
@wpietri wpietri temporarily deployed to Scheduled Testing June 11, 2025 17:57 — with GitHub Actions Inactive
@wpietri wpietri temporarily deployed to Scheduled Testing June 11, 2025 17:57 — with GitHub Actions Inactive
Copy link
Copy Markdown
Contributor

@bkorycki bkorycki left a comment

Choose a reason for hiding this comment

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

👍

@wpietri wpietri merged commit 692ba11 into main Jun 11, 2025
4 checks passed
@github-actions github-actions Bot locked and limited conversation to collaborators Jun 11, 2025
@wpietri wpietri deleted the empty-responses-are-safe branch January 16, 2026 14:08
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants