Skip to content

fix(vale-autofix): stop editing headings, scope commits to changed files - #1463

Merged
jth-nw merged 1 commit into
devfrom
fix/vale-autofix-scope-to-changed-files
Sep 1, 2026
Merged

fix(vale-autofix): stop editing headings, scope commits to changed files#1463
jth-nw merged 1 commit into
devfrom
fix/vale-autofix-scope-to-changed-files

Conversation

@jth-nw

@jth-nw jth-nw commented Sep 1, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • The auto-fix workflow's anchor-repair sweep rewrote heading links across every file in a product/version folder (or all of docs/kb/) whenever a heading got mechanically edited — this touched files the PR author never modified.
  • The final commit step (git add -A docs/) staged everything under docs/, not just the files the linters were scoped to.

Changes

  • scripts/vale-autofix.sh: Phase 1 now skips heading lines (same treatment as fenced code blocks), so mechanical fixes never rename a heading. Removed the now-unused anchor-sweep functions and --anchors-only mode.
  • .github/workflows/vale-autofix.yml: removed the "Fix heading anchors" step; Vale/Dale AI-fix prompts now explicitly skip any violation on a heading line; the commit step stages only files from changed-files.txt instead of git add -A docs/.
  • Replaced scripts/test-anchor-update.sh (tested the removed sweep) with scripts/test-heading-skip.sh, verifying Phase 1 leaves headings untouched while still fixing body text.

Test plan

  • npm run test:scripts passes (slugify, heading-skip, md-extension-autofix)
  • Open a test PR that trips a Vale/Dale rule on a heading line and confirm the heading is left alone and unrelated files aren't touched

🤖 Generated with Claude Code

… files

The anchor-repair sweep rewrote heading links across every file in a
product/version folder (or all of docs/kb/) whenever Phase 1 mechanically
changed a heading, and the final commit step staged all of docs/ rather
than just the files Vale/Dale/Claude were scoped to. Both let the
workflow push changes to files the PR author never touched.

Phase 1 now skips heading lines entirely, and the Vale/Dale AI phases are
instructed to skip heading violations, removing the need for the sweep.
The commit step now stages only the files in changed-files.txt.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Code Review

Reviewed the diff for correctness only (docs content/style is handled by the separate workflow). No blocking bugs found — the change is a net simplification and the two core mechanics are correct. A few non-blocking notes below.

Verified correct

  • Phase 1 heading skip (scripts/vale-autofix.sh:49) — the elif sits after the fence checks, so only non-fenced lines are tested. ^#{1,6}[[:space:]] correctly rejects #Heading (no space) and ####### seven (both non-headings in CommonMark), and heading-lookalikes inside fenced blocks are already caught by the earlier branches.
  • Scoped git add (.github/workflows/vale-autofix.yml:287-289) — safe under the step's default bash -e shell. When the last line of changed-files.txt is a file the PR deleted, [ -f "$file" ] fails and the loop returns 1, but per the Bash manual ("If a compound command other than a subshell returns a non-zero status because a command failed while -e was being ignored, the shell does not exit") the failing left side of an && list is an ignored context, so the step continues on to the git diff --cached check. Deleted paths are correctly skipped rather than erroring.
  • No dangling references--anchors-only, update_heading_anchors, and test-anchor-update.sh have no remaining callers in package.json, .github/, .husky/, scripts/, or .claude/. scripts/lib/slugify.sh is still needed by check-anchors.sh, so test-slugify.sh stays meaningful.
  • New test (scripts/test-heading-skip.sh) works standalone — vale-autofix.sh no longer needs a git repo or lib/slugify.sh, and absolute paths in the violations JSON are handled fine by the [ -f "$FILE" ] check. It does newly require jq on the test runner PATH (previously test-anchor-update.sh did not); fine on ubuntu-latest, worth knowing for local runs.
  • No changes to products.js, docusaurus.config.js, sidebars, or the KB copy pipeline. Workflow triggers, permissions:, and secret usage (VALE_TOKEN, ANTHROPIC_API_KEY, GITHUB_TOKEN) are untouched.

Notes

1. Nothing enforces the "never edit heading text" rule for Phases 2 and 3. Phase 1 skip is deterministic, but for the two Claude phases the only guard is now a prompt instruction (vale-autofix.yml:199, :244), and the sweep that used to repair the damage is gone. check-anchors.sh only validates outgoing links from the files it is given (check_file() scans hrefs in the source file), so if a Claude phase renames a heading anyway, links pointing into that heading from files outside changed-files.txt break, and the anchor-check step will not see it — the workflow commits and pushes, and the failure surfaces later as a build error. Consider a cheap deterministic guard instead of relying on the prompt: snapshot the heading lines per changed file before Phase 2, re-check after Phase 3, and revert (or fail the step on) any heading-line diff.

2. Author-renamed headings no longer get anchors repaired. The removed step diffed against origin/dev, so it also silently fixed incoming anchors when the PR author renamed a heading — not just when Phase 1 did. That behavior is gone now. Since onBrokenAnchors throws, the build still catches it, so this is a UX regression rather than a correctness hole, but it is a real change in what the workflow does for authors and worth a line in the PR description.

3. Out-of-scope edits are now discarded silently. If any phase modifies a file not in changed-files.txt, that edit is dropped at commit time with no signal — which is the intent, but it can leave a half-applied fix (for example, Phase 2 edits a link in file A and the target heading in unchanged file B; only A ships, so the pushed anchor is broken). A git status --porcelain -- docs/ echo after the git add loop would make any dropped edits visible in the job log.

4. Stale comments (nit). scripts/lib/slugify.sh:2 and scripts/test-slugify.sh:3 both still say the helper is sourced by vale-autofix.sh; it no longer is. Relatedly, the --test branch at scripts/vale-autofix.sh:9-13 is now dead — no test sources this script anymore.

@jth-nw
jth-nw merged commit c5b2f42 into dev Sep 1, 2026
11 checks passed
@jth-nw
jth-nw deleted the fix/vale-autofix-scope-to-changed-files branch September 1, 2026 16:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant