Skip to content

Stop treating pipeline-assigned fields as system_desc.json fields - #117

Open
arav-agarwal2 wants to merge 12 commits into
mainfrom
fix/drop-submission-id-from-system-desc
Open

arav-agarwal2 wants to merge 12 commits into
mainfrom
fix/drop-submission-id-from-system-desc

Conversation

@arav-agarwal2

@arav-agarwal2 arav-agarwal2 commented Oct 9, 2026 •

Copy link
Copy Markdown
Collaborator

Part of mlcommons/endpoints_policies#143. Rules side: mlcommons/endpoints_policies#144.

The pipeline assigns the submission ID (rules §8.5) and the submission and publication dates, and the bundle records the ID only as the <submission_id>/ directory (§8.1). The CLI still treated all three (submission_id, submission_date, publish_date) as system_desc.json fields, left over from v0.7:

  • Every bundled system_desc.json got all three as null. SystemDescription declared them, and the builder writes model_dump(), so the keys were added even when the submitter's file had none. (The submission_id entry is the one reported in the issue.)
  • A v0.7 numeric value failed system-description-valid, because the fields were typed str | None.
  • Points whose copies differ failed system-description-consistency, since the fields were compared like any system field.

Changes

  • New PIPELINE_ASSIGNED_FIELDS in models/file/system.py names the three keys. SystemDescription no longer declares them; extra="allow" still parses a leftover key of any type, and model_dump no longer invents them.
  • The builder drops them when loading a run's system_desc.json, next to where it already normalises division and the availability spellings.
  • The checker leaves them out of the consistency comparison (_IGNORED_SYSTEM_DESC_FIELDS), for bundles that reach it without going through the builder.

Nothing read these attributes on SystemDescription.

Testing

  • New tests in test_models.py, test_checker.py and test_builder.py, each run for all three fields. 21 of the 27 new cases fail on main (the other 6 pin that string and null values keep parsing); all pass here.
  • pytest: 1609 passed. ruff check src, ruff format --check on the changed files, and mypy are clean, and the Sphinx docs build has no new warnings.

🤖 Generated with Claude Code

https://claude.ai/code/session_01N2grVghMMtp7LPurYJ5o35

arav-agarwal2 and others added 6 commits October 9, 2026 17:09
The pipeline assigns the submission ID (rules 8.5), so it is not a 8.2 field.
Declaring it made model_dump write "submission_id": null into every bundled
system_desc.json and rejected a v0.7 numeric id.

Part of mlcommons/endpoints_policies#143.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01N2grVghMMtp7LPurYJ5o35
The bundle records the pipeline-assigned id as the <submission_id>/ directory,
so a value left in a run's system_desc.json from a v0.7 template is stale.

Part of mlcommons/endpoints_policies#143.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01N2grVghMMtp7LPurYJ5o35
A copy left over from a v0.7 template says nothing about the system, so points
whose copies differ no longer fail system-description-consistency.

Part of mlcommons/endpoints_policies#143.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01N2grVghMMtp7LPurYJ5o35
@github-actions

github-actions Bot commented Oct 9, 2026

Copy link
Copy Markdown

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

arav-agarwal2 and others added 6 commits October 9, 2026 17:21
The pipeline assigns them as it does the submission ID, so name all three in
PIPELINE_ASSIGNED_FIELDS for the builder and checker to share.

Part of mlcommons/endpoints_policies#143.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01N2grVghMMtp7LPurYJ5o35
@arav-agarwal2 arav-agarwal2 changed the title Stop treating submission_id as a system_desc.json field Stop treating pipeline-assigned fields as system_desc.json fields Oct 9, 2026
@anandhu-eng
anandhu-eng requested a balanced review from Copilot October 10, 2026 05:48

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟢 Approval recommended

The implementation consistently addresses legacy parsing, generated output, and cross-point comparison with comprehensive targeted tests.

0 open findings

What changed in this PR

Removes pipeline-assigned metadata from generated and validated system_desc.json files while preserving compatibility with legacy submissions.

Changes:

  • Defines and removes pipeline-assigned fields during bundle construction.
  • Excludes legacy values from consistency checks.
  • Adds model, checker, and builder coverage for all affected fields and value types.
File Description
src/​submission_checker/​models/​file/​system.py Removes declared pipeline fields and defines their shared names.
src/​submission_checker/​checker.py Ignores pipeline fields during consistency checks.
src/​endpoints_submission_cli/​submissions/​builder.py Drops stale pipeline fields before validation and output.
tests/​submission_checker/​test_models.py Tests parsing and serialization behavior.
tests/​submission_checker/​test_checker.py Tests validation and consistency behavior.
tests/​endpoints_submission_cli/​submissions/​test_builder.py Tests omission from generated bundles.

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

This branch has not been deployed

No deployments
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