Repository navigation
Conversation
extract_anchors_from_content only kept the bare slug, so links to the second and later copies of a repeated heading (#setup-1, #setup-2, …) failed the Link Checker even though GitHub generates those anchors.
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 addresses an issue where the link checker failed to validate internal links pointing to duplicate Markdown headings. By aligning the anchor extraction logic with GitHub's standard of using numeric suffixes for repeated headings, the changes ensure that internal links are correctly resolved and validated during CI processes. Highlights
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 the 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 counterproductive. 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
|
github-actions
Bot
added
the
status:awaiting review
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces GitHub-style duplicate heading slug numbering (e.g., setup, setup-1, setup-2) to the link checker tool and adds corresponding unit tests. The review feedback identifies a correctness issue where duplicate headings in Jupyter Notebooks are not tracked correctly across different cells because the state dictionary is initialized locally within the anchor extraction function. To resolve this, it is recommended to allow passing a shared state dictionary to preserve slug counts across cells and to add a test case verifying this behavior for notebooks.
Honor gemini-code-assist high-priority feedback on #1449: share seen_slugs across ipynb markdown/raw cells so GitHub-style setup / setup-1 numbering matches whole-document rendering, and add a cross-cell unit test. Signed-off-by: Mangesh Raut <[email protected]>
|
Thanks @mangeshraut712, correct fix. Verified: the #1437 fixture now yields setup / setup-1 and the audit exits 0, 21/21 tests pass with the 3 new ones failing on main, and a full internal audit of the repo is unchanged. The shared counter across notebook cells is the right call. LGTM. Heads-up: #1436 edits the same function and the same test region, so the two PRs conflict. Whichever merges second will need a quick rebase if #1436 lands first, could you rebase onto it? |
|
Friendly bump — CI is green and this is ready for review when you have a minute. Happy to adjust anything. |
|
Still green / merge-ready if you get a chance tonight. |
|
@kkorpal CI is green on this. It numbers duplicate heading anchors the same way GitHub does so the link audit stays consistent. You already verified the fixture — if it still looks good, could you please merge when you get a chance? Thanks. |
…peated-heading-anchors-1437 # Conflicts: # tools/check_all_links.py
Giom-V
left a comment
Giom-V
left a comment
There was a problem hiding this comment.
Thanks for the fix @mangeshraut712! I removed the unit test additions from this PR so we keep maintenance overhead low on internal helper scripts. Merging!
Why
extract_anchors_from_contentonly stored the bare heading slug. When a Markdown file had repeated headings, GitHub still exposes#setup-1,#setup-2, … for the later copies, but the Link Checker rejected those fragments and failed CI for valid section links. Fixes #1437.What
setup), the next issetup-1, thensetup-2, and so on.run_link_auditaccepting#setup-1.Test
python3 -m unittest tools.test_check_all_links -v— suite green, including the new duplicate-heading cases.## Setupheadings +[Second section](#setup-1)→ anchorssetup/setup-1,run_link_auditexits0(was1).