Skip to content

safety 1.3 id and annotation - #1693

Merged
bkorycki merged 2 commits into
mainfrom
safety-1.3-id
Oct 14, 2026
Merged

bkorycki merged 2 commits into
mainfrom
safety-1.3-id

Conversation

@bkorycki

@bkorycki bkorycki commented Oct 8, 2026

Copy link
Copy Markdown
Contributor

Preparing for next safety version

@bkorycki
bkorycki requested a review from a team as a code owner October 8, 2026 18:20
@bkorycki
bkorycki deployed to Scheduled Testing October 8, 2026 18:20 — with GitHub Actions Active
@github-actions

github-actions Bot commented Oct 8, 2026

Copy link
Copy Markdown

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

@bkorycki
bkorycki deployed to Scheduled Testing October 8, 2026 19:52 — with GitHub Actions Active
@bkorycki bkorycki changed the title safety 1.3 id safety 1.3 id and annotation Oct 8, 2026
class ViolationProbabilityAnnotation(BaseModel):
violation_probability: float | None
is_valid: bool
metadata: dict[str, Any] | None = None

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I feel like this is the kind of thing we're really trying to avoid. Does this really need to be an anything dict?

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, I think it does. This is where we dump all the node outputs from the annotator dag, which includes a variety of types that may not necessarily be the same across DAGs which produce this type of annotation. We follow the same structure in the existing SafetyAnnotation.

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.

Long term, I really don't like that we're digging the "mystery meat" hole deeper, but maybe it's necessary in the short term. What actually uses the annotation metadata downstream of this?

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.

Well I think the main idea is audit-ability. The exact outputs of each node are journaled so we have a good idea of what actually happened in the DAG. Also, some evaluator work involves analyzing per-node outputs of runs.

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.

In that case, let's go for it for now. Maybe we want to approach evaluator auditability a different way over the long term (e.g., explicit journaling that gets passed back), but I think this'll do for now.

@bkorycki
bkorycki merged commit ef0a9e5 into main Oct 14, 2026
3 checks passed

This branch was successfully deployed

1 active deployment
Scheduled Testing — 4c5266b2 Deployed Oct 8, 2026 by bkorycki via build #2595
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.

4 participants