Skip to content

Fix items with empty responses.#1521

Merged
superdosh merged 4 commits into
mainfrom
empty-response-fix
May 1, 2026
Merged

Fix items with empty responses.#1521
superdosh merged 4 commits into
mainfrom
empty-response-fix

Conversation

@superdosh
Copy link
Copy Markdown
Contributor

@superdosh superdosh commented May 1, 2026

Prior fix only updated the item's measurement, not the underlying annotations. This does that and also journals it as needed.

The other change is that the update is applied at the end of collect_annotations, NOT in handle_item anymore, so the measurement is done on the corrected item.

@superdosh superdosh temporarily deployed to Scheduled Testing May 1, 2026 14:10 — with GitHub Actions Inactive
@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 1, 2026

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

@superdosh superdosh temporarily deployed to Scheduled Testing May 1, 2026 14:39 — with GitHub Actions Inactive
@superdosh superdosh force-pushed the empty-response-fix branch from 4cf6536 to 6114145 Compare May 1, 2026 14:41
@superdosh superdosh temporarily deployed to Scheduled Testing May 1, 2026 14:41 — with GitHub Actions Inactive
@superdosh superdosh marked this pull request as ready for review May 1, 2026 14:43
@superdosh superdosh requested a review from a team as a code owner May 1, 2026 14:43
@superdosh superdosh temporarily deployed to Scheduled Testing May 1, 2026 14:57 — with GitHub Actions Inactive
@superdosh superdosh had a problem deploying to Scheduled Testing May 1, 2026 15:50 — with GitHub Actions Failure
@superdosh superdosh temporarily deployed to Scheduled Testing May 1, 2026 15:58 — 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.

looks good for an immediate fix.

# log if any annotations were overridden
if overridden_annotators:
self.test_run.journal.item_entry(
"overrode item annotation",
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.

Does the consistency checker reference journal entries by label? If so, it may be good to check if changing "overrode item quality" to "overrode item annotation" will break anything there.

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.

The only place I see "overrode" in the code base is here and in the test also in this PR. So I don't think anything else references it. That seems safe right?

# overwrite the annotations as unsafe
for annotator_uid in pipeline_item.annotations:
pipeline_item.annotations[annotator_uid].is_safe = False
# re-run the handle, which should populate the measurement (and fix)
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.

👍🏻

@superdosh superdosh merged commit 5dd6d4c into main May 1, 2026
3 checks passed
@superdosh superdosh deleted the empty-response-fix branch May 1, 2026 17:04
@github-actions github-actions Bot locked and limited conversation to collaborators May 1, 2026
@superdosh
Copy link
Copy Markdown
Contributor Author

Follow up here: https://github.com/mlcommons/sugar/issues/477

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