Fix update-copyright swallowing a doc comment after the header#33
Merged
alexander-yevsyukov merged 2 commits intoJul 17, 2026
Merged
Conversation
For hash-style (#) files, strip_existing_header consumed blank lines as well as comment lines while looking for the end of the header. When a doc comment immediately followed the license header separated only by a blank line (no secondary "#" line), the loop kept going into the doc comment and treated the whole thing as one header block, silently deleting the doc comment on the next re-stamp. Blank notice-paragraph separators always render as a bare "#", never as a truly empty line (see build_header), so a genuinely blank line reliably marks the end of the header. Stopping there instead of also consuming blank lines fixes the bug; strip_leading_blank_lines still consumes the separator before the body content afterwards. Found while updating copyright years to 2026 across the config repo, where it had silently deleted documentation from 7 files. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes a data-loss edge case in the update-copyright script’s hash-style (#) header stripping logic, ensuring that a real blank line ends the header so subsequent doc comments are preserved during re-stamping.
Changes:
- Adjust
strip_existing_header()forhashstyle to stop consuming at the first genuinely blank line (instead of treating blank lines as part of the header block). - Add a regression test covering a hash-style header followed by a blank line and then a doc comment, ensuring the doc comment is not deleted.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| skills/update-copyright/scripts/update_copyright.py | Fixes hash-style header detection to stop at a truly blank line, preventing accidental consumption of following doc comments. |
| skills/update-copyright/tests/test_update_copyright.py | Adds a regression test that reproduces and guards against the doc-comment deletion scenario. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 10a3b3aefb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
5 tasks
The previous fix stopped the header-detection loop at the first genuinely blank line unconditionally. That missed legacy/hand-written hash-style headers that use a real blank line as their own internal paragraph separator (e.g. between "Copyright ..." and "All rights reserved") rather than the bare "#" a freshly rendered header uses: the loop now stopped before "Licensed under"/"All rights reserved" ever appeared, so is_copyright_header() never matched and the stale header was silently left unchanged. Only treat a blank line as the end of the header once the text accumulated so far already satisfies is_copyright_header; otherwise keep consuming, matching the original behavior for such headers while still stopping before an unrelated doc comment that follows a well-formed header. Reported by Codex review on #33. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
armiol
approved these changes
Jul 17, 2026
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
skills/update-copyright/scripts/update_copyright.py: for hash-style (#) files where a doc comment immediately follows the license header (separated only by a blank line, no secondary#separator line),strip_existing_headerkept consuming through the blank line and swallowed the doc comment as part of the header block, silently deleting it on re-stamp.#(seebuild_header), never as a truly empty line, so a real blank line reliably marks the end of the header.strip_leading_blank_linesstill consumes the separator before the body content afterwards.test_hash_header_update_preserves_doc_comment_after_blank_line) reproducing the exact shape that triggered the bug.Context
Found while updating copyright years to 2026 across the
configrepo — the unfixed script had silently deleted documentation from 7 files (gcs.properties,scripts/decrypt.sh,scripts/ensure-reports-updated.sh,scripts/publish-artifacts.sh,scripts/request-package-names.sh,scripts/trigger-publishing.sh,scripts/update-copyright.sh), caught byspine-code-reviewand fixed there in config#735. This PR lands the actual root-cause fix so the same bug doesn't recur on the next annual bump in any repo that floats this shared script.Test plan
python3 -m unittest test_update_copyright— 9/9 pass, including the new regression test.master(44d5290).🤖 Generated with Claude Code