[raycicmd] Add validation guards for array expansion#470
[raycicmd] Add validation guards for array expansion#470andrew-anyscale merged 1 commit intomainfrom
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances the robustness of the array expansion logic within Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces several validation guards to the array expansion logic, which is a great improvement for robustness. The changes detect duplicate base keys, duplicate elements from adjustments, and key collisions after sanitization, all of which will help catch configuration errors early. The implementation is clean and accompanied by thorough tests for each new validation, as well as a test for partial-dimension skip behavior. Overall, this is a solid contribution that enhances the reliability of the pipeline generation.
b2ec364 to
2ca9c57
Compare
Detect three classes of errors early during array expansion: - Duplicate base key: two array steps sharing the same key/name silently overwrote configs - Duplicate element from adjustment: an addition adjustment that duplicates an existing element would produce duplicate pipeline steps - Key collision from sanitization: values like "1.2.1" and "121" that produce the same sanitized key part Also extracts depends_on resolution into array_depends.go (arraySelector, parsing, resolution) to keep array_expand.go focused on step expansion. Also adds a test for partial-dimension skip behavior (skip by one dim removes all matching combinations). Topic: array-validation-guards Signed-off-by: andrew <andrew@anyscale.com>
2ca9c57 to
f649330
Compare
aslonnie
left a comment
There was a problem hiding this comment.
this seems to be mostly moving code around? which part is new/changed?
|
This was a bad commit smash that put this + another refactor into one. The actual validation guards are: |
Detect three classes of errors early during array expansion:
Also extracts depends_on resolution into array_depends.go (arraySelector, parsing, resolution) to keep array_expand.go focused on step expansion.
Also adds a test for partial-dimension skip behavior (skip by one dim removes all matching combinations).
Topic: array-validation-guards
Signed-off-by: andrew andrew@anyscale.com