-
Notifications
You must be signed in to change notification settings - Fork 247
config: make aggregate multinode topology explicit #2479
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
Open
cquil11
wants to merge
1
commit into
agent/node-slot-scheduling-poc
from
agent/explicit-aggregate-node-counts
Open
Changes from all commits
Commits
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.
🟡 PR title and description are English-only, missing the Simplified Chinese translation that AGENTS.md mandates for every PR (
<English title> / <中文标题>and a## 中文说明body section). This is a pre-existing repo convention this PR doesn't follow; please add the Chinese translation before merging.Extended reasoning...
AGENTS.md line 7 states an explicit, non-negotiable convention: 'PR and GitHub-issue titles & descriptions must be bilingual — include a Simplified Chinese version in addition to English. Title format:
<English title> / <中文标题>. In the PR/issue body, follow the English content with its Chinese translation (e.g. a## 中文说明section mirroring the summary...).' It goes on to say 'This applies to every PR and every issue.'PR #2479's title is
config: declare aggregate multinode allocation sizes— English only, with no trailing/ <中文标题>segment. The description body (Summary + Validation sections) contains no Chinese text at all, and specifically no## 中文说明section as the convention requires.Step-by-step verification:
config: declare aggregate multinode allocation sizes— no/ <中文>suffix present.## Summaryand## Validationsections, both English, no## 中文说明or any Chinese characters.Nothing in the diff or the PR's actual content (a YAML config change backfilling
num-nodesfields) is affected by this — the change itself is fine. The gap is purely in the PR metadata (title/description), which the author can fix without touching code: rename the title to append/ <中文标题>and add a## 中文说明section mirroring the Summary.This does not cause any runtime failure, benchmark regression, or data-correctness issue — it's a process/metadata compliance gap against a documented repo convention, not a code defect. Per the review rubric, a normal-severity finding should represent something that fails or breaks if unaddressed; this doesn't. It also isn't a PR-description-mismatch case (the description isn't inaccurate, it's just monolingual), so it's evaluated directly against the AGENTS.md rule rather than downgraded under that heuristic. Given AGENTS.md explicitly designates this as mandatory for every PR, it still merits being called out, but as a nit the author can resolve in under a minute rather than a blocking issue.