-
Notifications
You must be signed in to change notification settings - Fork 155
dsv4-fp4-b300-sglang: revert to #1143 low-latency-only baseline #1184
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
cquil11
merged 3 commits into
main
from
chore/revert-dsv4-fp4-b300-sglang-to-1143-baseline
Apr 26, 2026
Merged
Changes from all commits
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔴 This PR deletes six prior
dsv4-fp4-b300-sglangchangelog entries fromperf-changelog.yaml, bututils/process_changelog.py:get_added_linesexplicitly raisesValueErroron any non-whitespace deletion in this file. Therun-sweepworkflow invokes that script on every PR with a sweep label and on every push to main, so the very first deletion (- config-keys:) will abort the workflow before any benchmark runs — defeating the PR's stated test plan. Fix by dropping the wipe and only appending the new entry.Extended reasoning...
What the bug is
utils/process_changelog.pyenforces an append-only invariant onperf-changelog.yaml. The relevant code atutils/process_changelog.py:24-37(get_added_lines) iterates over the unified diff for the changelog file and explicitly rejects any non-whitespace deletion:This PR's diff does the opposite: it removes six prior
dsv4-fp4-b300-sglangentries (PRs #1143, #1132, #1158, #1173, #1174, #1178) spanning lines 1779-1793 and 1838-1855 in the diff. There are roughly 30 non-whitespace deleted lines (- config-keys:,- dsv4-fp4-b300-sglang, the description bullets, andpr-link: ...), all of which strip to non-empty content.Why this is a hard CI blocker
The validation is wired into
.github/workflows/run-sweep.yml:91-101(thesetupjob), which callsprocess_changelog.pywhenever the workflow runs. The workflow runs in two contexts that matter here:sweep-enabled/full-sweep-enabledlabel — required to actually exercise the sweep this PR is asking for.[skip-sweep].In both contexts, the deletion check fires before any pydantic
ChangelogEntryvalidation. There is no override flag.Proof: walking through the diff
Take the first non-whitespace deletion in the diff, around line 1782:
When
get_added_linesprocesses this:-- config-keys:→ starts with-, doesn't start with---;deleted_content = "- config-keys:",.strip()is non-empty →ValueErrorraised on the very first iteration.The setup job exits non-zero, no benchmark or sweep job runs, and the PR's test plan ("Sweep run on B300 completes end-to-end") cannot be satisfied. Post-merge, the same failure occurs on the push-to-main run, so the new entry never triggers a sweep — directly defeating the PR's purpose.
What contradicts existing safeguards
The PR description literally says "Wipes all six prior dsv4-fp4-b300-sglang changelog entries (from #1143, #1132, #1158, #1173, #1174, #1178) and adds a single fresh entry to start clean." This is in direct conflict with the append-only invariant the code enforces. This bug is independent of any
pr-linkissue — even if the new entry were perfect, the deletions would still tripget_added_linesfirst.How to fix
Two options:
[changelog-rewrite]commit-message marker, or asquash:block in the YAML), but this is a larger policy change and should not be smuggled in here.Option (1) is the obviously safe path.