-
Notifications
You must be signed in to change notification settings - Fork 270
[AMD][MI35X] 0821 deepseek-v4 sglang agentic benchmark #2705
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
Closed
+11
−1
Closed
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
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.
🟡 AGENTS.md declares perf-changelog.yaml append-only and byte-sensitive ('Preserve all existing bytes and separator whitespace, and append only at the tail'), but this PR rewrites the prior trailing separator line (two trailing spaces after the PR #2643 entry) into a bare empty line before appending the new entry. Please restore the original ' ' (two-trailing-space) separator line and append the new entry after it, untouched.
Extended reasoning...
What happened:
perf-changelog.yamlis explicitly declared inAGENTS.md(line 21) as append-only and byte-sensitive: 'Preserve all existing bytes and separator whitespace, and append only at the tail.' This PR's diff to that file is not a pure append — it modifies the very last line of the pre-existing file content before adding the new entry.The exact diff hunk:
Step-by-step proof:
git show HEAD~1:perf-changelog.yaml | tail -n 2 | cat -Aon the pre-PR tree shows the file's last line is$— i.e. a line containing exactly two trailing space characters, no other content.-line containing those same two spaces (-) being removed, and replaced with a+line that is completely empty (+).- config-keys: ...entry is appended.git show HEAD:perf-changelog.yamlshows that line is now bare (no trailing spaces) where it previously carried two.Why this matters: The repo instructions call this out under 'Non-negotiable benchmark invariants,' explicitly to protect against this exact class of edit — presumably because some tooling or diffing process downstream depends on byte-stable history for this file (e.g. line-count/byte-offset based diffing across changelog entries, or simply to keep git blame/diff noise-free per entry). Even though this specific alteration is whitespace-only and doesn't break YAML parsing, it is a real, literal violation of an explicit rule the repository asks reviewers to enforce, and it was introduced fresh by this PR (the byte was fine at HEAD~1).
How to fix: When appending the new changelog entry, avoid touching the previous entry's trailing separator line. Concretely, the append should read as pure addition after the existing bytes (including the two trailing spaces), for example using
printf '...' >> perf-changelog.yamlor an editor mode that never rewrites already-committed lines, rather than a tool that reformats/strips trailing whitespace across the whole file (e.g. some YAML formatters or editors with 'trim trailing whitespace on save' enabled).