Skip to content

[yamllint-fixer] Fix yamllint comments-indentation for fully-commented on: children - #46960

Merged
pelikhan merged 2 commits into
mainfrom
fix/yamllint-deployment-status-comments-indentation-6c871a90fb89b743
Jul 21, 2026
Merged

[yamllint-fixer] Fix yamllint comments-indentation for fully-commented on: children#46960
pelikhan merged 2 commits into
mainfrom
fix/yamllint-deployment-status-comments-indentation-6c871a90fb89b743

Conversation

@github-actions

@github-actionsgithub-actionsBot commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes a yamllint comments-indentation warning that appeared when an on: event key (e.g. deployment_status:) had all its children commented out. In that case the previous logic kept the comment block at its original 4-space indent, which violated yamllint's rule because the nearest real content line is the shallower parent key. The fix dedents those fully-commented blocks to column 0.

When at least one real sibling line exists at the same indentation (e.g. types: alongside # forks:), the block is left untouched as before.

Changes

pkg/workflow/frontmatter_on_section_cleanup.go

  • Replaced the unconditional early-return for deeply-indented trailing comment blocks with an anchor-based check.
  • Walks backward to find the nearest non-blank preceding line; if that anchor is at a strictly shallower indent than the comment block (i.e. it is the parent key, not a sibling), the block is dedented to column 0.
  • A sibling at the same or deeper indent still short-circuits dedenting, preserving existing behaviour.

pkg/workflow/frontmatter_on_section_cleanup_test.go

  • Adds "deeply nested trailing block whose parent has no real children is dedented" — verifies deployment_status: with only commented-out children is dedented.
  • Adds "deeply nested trailing block with a real sibling at its indent is left untouched" — verifies pull_request: with both types: and commented # forks: retains indentation.

pkg/workflow/compiler_forks_test.go

  • Updates three test-fixture expected YAML strings to reflect the corrected column-0 dedent for fork-filter comment blocks that are the sole children of pull_request:.

Motivation

The yamllint comments-indentation rule requires comments to align with the content that follows them. A deeply-indented comment block with no real sibling content has no valid anchor at that indent level; dedenting to column 0 matches the top-level content that follows the on: section and satisfies the linter.

Testing

All changes are covered by updated and new unit tests in frontmatter_on_section_cleanup_test.go and compiler_forks_test.go.

Generated by PR Description Updater for #46960 · 30.8 AIC · ⌖ 4.54 AIC · ⊞ 4.8K ·

When every child of an `on:` event key (e.g. `deployment_status:`) is
commented out, the trailing comment block sat at the child indent (4)
while its parent key was at indent 2 and the following top-level key at
column 0, tripping yamllint's comments-indentation rule.
dedentTrailingOnCommentBlock now dedents a deep trailing block to column 0
when its nearest real content line is a shallower parent key (no sibling
to anchor to), while still preserving nested-event blocks that have a real
sibling at their indent.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@pelikhan
pelikhan marked this pull request as ready for review July 21, 2026 04:07
CopilotAI review requested due to automatic review settings July 21, 2026 04:07
@github-actions

github-actionsBot commented Jul 21, 2026

Copy link
Copy Markdown
ContributorAuthor

Design Decision Gate 🏗️ completed the design decision gate check.

No ADR enforcement needed: PR #46960 does not have the 'implementation' label and has only 62 new lines of code in business logic directories (threshold: 100).

@github-actions

github-actionsBot commented Jul 21, 2026

Copy link
Copy Markdown
ContributorAuthor

PR Code Quality Reviewer completed the code quality review.

@github-actions

github-actionsBot commented Jul 21, 2026

Copy link
Copy Markdown
ContributorAuthor

Test Quality Sentinel completed test quality analysis.

@github-actions

github-actionsBot commented Jul 21, 2026

Copy link
Copy Markdown
ContributorAuthor

🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Refines YAML generation to dedent fully commented nested on: children and eliminate yamllint indentation warnings.

Changes:

  • Detects fully commented nested event parents.
  • Adds regression coverage for dedented and preserved blocks.
Show a summary per file
FileDescription
pkg/workflow/frontmatter_on_section_cleanup.goRefines trailing-comment dedentation.
pkg/workflow/frontmatter_on_section_cleanup_test.goAdds regression cases.

Review details

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 2/2 changed files
  • Comments generated: 1
  • Review effort level: Medium

Comment on lines +775 to +776
if anchorIndent >= firstLineIndent {
return lines

@github-actionsgithub-actionsBot left a comment

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Logic is correct and well-tested. The anchor-based check cleanly distinguishes the fully-commented-parent case (dedent) from the real-sibling case (preserve), and the two new regression tests cover both paths. No issues found.

🧵 Reviewed using Impeccable skills by Impeccable Skills Reviewer · 13 AIC · ⌖ 4.99 AIC · ⊞ 5K

@github-actionsgithub-actionsBot left a comment

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Skills-Based Review 🧠

Applied /diagnosing-bugs and /tdd — approving with one minor observation.

📋 Key Themes & Highlights

Key Themes

  • Root cause vs symptom: The fix correctly targets the real problem (over-broad early-return in dedentTrailingOnCommentBlock) rather than suppressing the yamllint warning downstream.
  • Anchor heuristic: Looking at the nearest preceding non-blank line and comparing indentation levels is simple and effective for the described cases.
  • Test coverage: Both the regression case (fully-commented parent → dedent) and the guard case (real sibling → no dedent) are covered.

Positive Highlights

  • ✅ Clear code comments explaining the before/after semantics
  • ✅ Both the bug case and the adjacent "preserve" case are tested
  • ✅ The logic change is minimal and surgical

Observation

The anchor walk (anchor-- loop) can cross outside the on: block if the trailing comment block starts early. In practice this is safe because an on: key itself sits at indent 0, which is always shallower than the comment indent, so the heuristic still dedents correctly. But if the section ever gains a preceding non-blank line that happens to be at the same indent as the comment (e.g. another deeply-nested event key), the guard would fire and leave the comments un-dedented. Consider adding a comment noting this assumption, or bounding the anchor search to the section's start index.

🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · 21.1 AIC · ⌖ 4.47 AIC · ⊞ 6.7K
Comment /matt to run again

@github-actionsgithub-actionsBot mentioned this pull request Jul 21, 2026
@github-actions

Copy link
Copy Markdown
ContributorAuthor

🧪 Test Quality Sentinel Report

Test Quality Score: 100/100 — Excellent

Analyzed 2 test(s): 2 design, 0 implementation, 0 violation(s).

📊 Metrics (2 tests)
MetricValue
Analyzed2 (Go: 2, JS: 0)
✅ Design2 (100%)
⚠️ Implementation0 (0%)
Edge/error coverage2 (100%)
Duplicate clusters0
InflationNo (1.21:1)
🚨 Violations0
TestFileClassificationIssues
deeply nested trailing block whose parent has no real children is dedentedfrontmatter_on_section_cleanup_test.go:122Design + Edge✅ Validates new algorithm path
deeply nested trailing block with a real sibling at its indent is left untouchedfrontmatter_on_section_cleanup_test.go:139Design + Boundary✅ Critical negative case

📋 Analysis Details

Test Structure:

  • Table-driven subtests within TestDedentTrailingOnCommentBlock
  • Defensive copy pattern to prevent slice corruption (line 190-191)
  • Comprehensive assertions: length check + line-by-line exact match with context
  • Descriptive failure messages with full output on mismatch

Design Invariants Verified:

  1. ✅ Comments after a parent key with no real children dedent to column 0 (fixes yamllint's comments-indentation warning)
  2. ✅ Comments with real sibling at same indent preserve indentation (doesn't over-dedent)
  3. ✅ Multiline comment blocks are handled uniformly

Quality Signals:

  • 0% implementation tests (2/2 are design-level)
  • 100% edge/error coverage (boundary case + positive case)
  • No code smell: no Go mocks, no build tag violations, defensive patterns present
  • Test inflation: 1.21:1 (34 test lines / 28 prod lines) — well within acceptable range

Verdict

Passed. 0% implementation tests (threshold: ≤30%). No violations.

Both new tests are high-value, precise design-level tests that directly validate the bug fix and its critical boundary condition. The table-driven structure and comprehensive assertions provide strong confidence in the behavior.

🧪 Test quality analysis by Test Quality Sentinel · 16.7 AIC · ⌖ 9.78 AIC · ⊞ 7K ·
Comment /review to run again

@github-actionsgithub-actionsBot left a comment

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Test Quality Sentinel: 100/100. 0% implementation tests (threshold: ≤30%). All tests are high-value design-level validations of the bug fix and boundary conditions.

@github-actionsgithub-actionsBot left a comment

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clean fix. The anchor-walk logic is correct — the start-walk at line 738 already guarantees lines[start-1] is a non-blank, non-comment real content line, so the anchor blank-skip loop in the new branch is simply extra safety. Both test cases cover the two new branches precisely.

Minor: tab/space byte-count assumption (pre-existing)

firstLineIndent and anchorIndent are computed as raw byte lengths via len(s) - len(strings.TrimLeft(s, " \t")), so a tab counts as 1, not 8. Mixed tab/space indentation in the same on: block would produce a wrong anchorIndent >= firstLineIndent comparison. This assumption is shared with the existing firstLineIndent <= 2 path above and is not introduced by this PR; mixed indentation in GitHub Actions YAML is virtually nonexistent in practice. Not blocking.

🔎 Code quality review by PR Code Quality Reviewer · 35.2 AIC · ⌖ 4.4 AIC · ⊞ 5.6K
Comment /review to run again

@pelikhan

Copy link
Copy Markdown
Collaborator

@copilot run pr-finisher skill

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

CopilotAI commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

@copilot run pr-finisher skill

Fixed in 63ba691. The reviewer was correct — the new dedent condition also matched pull_request with a forks-only child (both structurally have the parent event key at indent 2 and the comment block at indent 4). Since yamllint would flag # forks: at indent 4 for the same reason as # state: (the preceding real content is at indent 2 and the following top-level content is at column 0), the right fix is to update the test fixtures to expect column-0 comments for the three forks-only cases rather than narrow the condition. All four TestForksArrayFieldCommentingInOnSection sub-tests now pass.

CopilotAI requested a review from pelikhanJuly 21, 2026 04:52
@pelikhan
pelikhan merged commit 5cb9f62 into mainJul 21, 2026
23 checks passed
@pelikhan
pelikhan deleted the fix/yamllint-deployment-status-comments-indentation-6c871a90fb89b743 branch July 21, 2026 05:12
@github-actions

Copy link
Copy Markdown
ContributorAuthor

🎉 This pull request is included in a new release.

Release: v0.82.15

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@pelikhan