[dstrodtman]

Why

rayci selects steps by tag and passes if: through to Buildkite without evaluating it. A step gated on a PR label, such as if: '!(build.pull_request.labels includes "docstring-only")', is dropped by Buildkite after upload, but rayci has already marked its depends_on chain, so its images still build. Gating also has to be written on every step: extending Ray's docstring-only label past Ray Data would mean a negated if: on nearly every premerge test step, and a new step that forgets one silently runs under the label.

This is the first of two stacked PRs that let PR labels adjust tag selection before dependency marking. This one adds the keep-only form. The second adds skip forms.

What changes

A *.labels.yaml file in a Buildkite directory declares rules keyed by PR label, discovered the way *.rules.txt files are:

- label: docstring-only
  keep_only_tags: [always, lint, docstring_examples]

When a rule's label is in BUILDKITE_PULL_REQUEST_LABELS, a step is selected only if it would be selected anyway and it carries one of the rule's keep_only_tags. The condition is a new acceptKeepOnly check alongside the existing tag and select checks in stepFilter.accept.

• Dropped steps aren't rejected, so they don't go through rejectDeps. Images that only dropped steps need are no longer marked. A step that a kept step depends on still runs, with its images.
• It applies under run-all (*) too, so a labeled PR that also trips a run-all rule stays narrowed.
• It applies to group nodes through the same accept path, so a group whose own tag is selected but not kept no longer accepts all its steps.
• Untagged steps aren't kept, even with no_tag_means_always.
• When several keep-only rules apply, a step needs a tag from any of them.
• Each rule that fires logs a line next to the existing selected tags: line, such as label "docstring-only": keep only tags: always lint docstring_examples, so the upload log shows why steps dropped.
• With no *.labels.yaml file, nothing changes and the labels variable isn't read.

Why a repo file rather than built-in config

Ray's config, including skip_tags, is compiled into rayci (builtin_configs.go). Rules there would need a rayci release and a .rayciversion bump for every label change. A file in the repo's .buildkite/ keeps label policy next to the tag rules it modifies, under the same CODEOWNERS review. The file name and format are open to change.

Testing

• Unit tests for parsing and validation, label parsing, rule application, stepFilter keep-only including run-all and untagged steps, convertGroups (dropped-only images drop, shared images stay, a kept step's non-kept dependency runs), and an end-to-end makePipeline test with a discovered rules file.
• go test ./..., go fmt, pre-commit, and rayci-lint go-filelength and go-coverage pass. raycicmd coverage is 90.4%.
• Simulated Ray premerge with a Linux build of this branch against ray-project/ray master at 8fdacd39fd4, plus a probe commit that adds the rule above and tags doc: docstring examples with docstring_examples:

Simulated PR	Unlabeled	docstring-only
Core docstring (python/ray/actor.py)	165 steps	13 steps, 5 image jobs
Ray Data docstring (python/ray/data/dataset.py)	51 steps	13 steps, 5 image jobs

Unlabeled output is identical to rayci v0.50.0. The 13 labeled steps are test-rules, the lint steps, the docstring-only scope guard, doc: docstring examples, and their images. Which further steps to keep, such as the API doc consistency check, is a Ray-side choice made when Ray adopts the rule.

Questions for review

• Is a repo-owned *.labels.yaml file the right place, or should rules live in the built-in config?
• Should keep-only apply under run-all? This PR says yes.

Follow-up

The stacked PR adds skip_tags_unless_present and skip_tags_when_present. After a release, Ray bumps .rayciversion, adds its rules file, and removes the per-step label if: gates.

🤖 Generated with Claude Code

[gemini-code-assist]

Code Review

This pull request introduces label-based step filtering rules to adjust pipeline step selection when a pull request carries specific labels. It adds support for parsing label rules from YAML files, loading them, and applying them to the step filter. The review feedback points out that the YAML parser does not gracefully handle empty files or null elements in the rules list, which could lead to unexpected errors or nil pointer dereferences. It is recommended to add checks for empty content and non-nil rules, along with corresponding test cases to ensure robust parsing.

[gemini-code-assist]

The YAML parser does not handle empty files or null elements in the rules list gracefully. An empty file causes yaml.NewDecoder.Decode to return io.EOF, resulting in an unexpected error. Additionally, if the YAML list contains a null element, it is unmarshaled as a nil pointer, which will cause a panic (nil pointer dereference) when checking r.Label. Checking for empty content and verifying that each rule is non-nil prevents these issues.

func parseLabelRulesFile(file string) ([]*labelRule, error) {
	bs, err := os.ReadFile(file)
	if err != nil {
		return nil, fmt.Errorf("read label rules file: %w", err)
	}

	if len(bytes.TrimSpace(bs)) == 0 {
		return nil, nil
	}

	var rules []*labelRule
	dec := yaml.NewDecoder(bytes.NewReader(bs))
	dec.KnownFields(true)
	if err := dec.Decode(&rules); err != nil {
		return nil, fmt.Errorf("unmarshal label rules file %s: %w", file, err)
	}

	for i, r := range rules {
		if r == nil {
			return nil, fmt.Errorf("label rule %d in %s: rule is null", i, file)
		}
		if r.Label == "" {
			return nil, fmt.Errorf("label rule %d in %s: label is empty", i, file)
		}
		if len(r.KeepOnlyTags) == 0 {
			return nil, fmt.Errorf(
				"label rule %q in %s: no tags to keep", r.Label, file,
			)
		}
	}
	return rules, nil
}

[dstrodtman]

Fixed in 6a6ca5e, with one change from the suggestion. Trimming whitespace would still fail on a comment-only file, which decodes to io.EOF too, so the parser treats io.EOF from Decode as no rules instead. A null entry now returns label rule N in <file>: rule is null.

[gemini-code-assist]

To ensure robust handling of empty files and null elements in the YAML configuration, we should add corresponding test cases to the test suite.

func TestParseLabelRulesFile_errors(t *testing.T) {
	for _, test := range []struct {
		name    string
		content string
	}{{
		name:    "empty label",
		content: "- keep_only_tags: [always]",
	}, {
		name:    "no tags",
		content: "- label: docstring-only",
	}, {
		name:    "unknown field",
		content: "- label: docstring-only\n  keep_tags: [always]",
	}, {
		name:    "null element",
		content: "- label: docstring-only\n  keep_only_tags: [always]\n- null",
	}} {
		t.Run(test.name, func(t *testing.T) {
			p := writeLabelRules(t, t.TempDir(), test.content)
			if _, err := parseLabelRulesFile(p); err == nil {
				t.Errorf("parseLabelRulesFile(%q) = nil error, want error", test.content)
			}
		})
	}
}

func TestParseLabelRulesFile_empty(t *testing.T) {
	p := writeLabelRules(t, t.TempDir(), "")
	got, err := parseLabelRulesFile(p)
	if err != nil {
		t.Fatalf("parseLabelRulesFile with empty file: %v", err)
	}
	if got != nil {
		t.Errorf("parseLabelRulesFile with empty file = %v, want nil", got)
	}
}

[dstrodtman]

Added in 6a6ca5e: a null element case in TestParseLabelRulesFile_errors, and TestParseLabelRulesFile_noRules covering empty, newline-only, and comment-only files.

[github-actions]

⚠️ PR Size Warning

This PR exceeds the size limits defined in .buildkite/raycilint.yaml.

• additions: 624 (max allowed: 500)

Break this into smaller PRs, or apply the skip-prcheck label to bypass.