-
Notifications
You must be signed in to change notification settings - Fork 46
fix(#2069): Add Render Hook for Automatic Link Checking for Markdown Links #2094
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(#2069): Add Render Hook for Automatic Link Checking for Markdown Links #2094
Conversation
|
With the hook running my docker container for cht-docs errors out because of links that the hook can't find. That's good, but some of the failures seem to be because the render link hook does not handle aliases. I would like guidance on how to proceed. List of errors that the hook generatedExpand to see the error outputI have been spot-checking but have not checked them all. I don't know yet how many are the hook not checking aliases. You can also see them in CI: https://github.com/medic/cht-docs/actions/runs/19934828879/job/57156581913?pr=2094 The problem with marking aliases as errorsFrom what I can tell, the render hook uses Hugo's {{- with $p := or ($.PageInner.GetPage .) ($.PageInner.GetPage (strings.TrimRight "/" .)) }}The Here's an example error that the custom hook generates: When I click on the link for in Possible solutionsOption 1: Replace aliases with actual page paths. We can update the links that generate the error so that we don't use aliases. Option 2: Update the hook to search for aliases. I believe we can adjust the hook to also search for aliases, but that may slow down the build time. I think it would require the hook to iterate through all site pages for every link that doesn't resolve directly. |
|
@cliftonmcintosh - your detailed and helpful write up of where you're stuck has a perfect eye for detail and explicit request for clarification. When coupled with two solutions, it makes for a tremendously actionable and async friendly PR. Thank you!! To the matter at hand, I think we should pay the one time cost of fixing all the aliases. This will make it faster (vs option 2), will create less aliases of aliases of aliases links over time and will prevent the same for larger refactors in the future. As such, I'll help fix all the links in this PR as I can go through them pretty quick and know the content decently well. I'll try and get this done in the next day or so as a PR against your branch. cc @andrablaj |
|
Oh wow - I replaced all the Well - the only way to get out of here is through fixing all the links just this this once 🚀 |
😮 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@cliftonmcintosh - this is looking really good!
I know this is a WIP PR and we're still waiting for me to tidy up all the links, but giving some early feedback to be sure both linkErrorLevel and highlightBroken work.
While I was able to easily change linkErrorLevel to the hugo.toml in the [params] section and got WARN instead of ERROR in the log, I was unable to get the highlightBroken classes to show on the broken links.
As well, we'll need to add the corresponding CSS:
a.broken {
background: #ff0;
border: 2px solid #f00;
padding: 0.1em 0.2em;
}Finally, we should be sure to document how these two features work, as it greatly eases the pain of needing to restart hugo every time you fix a broken link. Included should obviously be the the importance of NOT committing linkErrorLevel = "WARN" to main 😅 .
I'm super excited to see this through - I think these tight constrains on link integrity will be really great long term!
|
Ohh - yeah - this all looks great - thanks @cliftonmcintosh ! I'll spin up another round of link fixing tomorrow-ish, but I'll first be sure to merge your latest changes back to my branch and follow these steps. Thanks and stay tuned!! |
…ot-working-with-markdown-links' into 2069-automatic-link-checking-not-working-with-markdown-links
|
@cliftonmcintosh - I merged your latest changes to my branch and I'm still stuck on it not ever starting the web server. I'll think on this some more and have reached out to some colleagues to help debug, but feel free to dive in if you're feeling ambitious! |
mrjones-plip
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@cliftonmcintosh - yay! Great team work on this.
The one issue I found is it breaks on certain links. Can you reproduce and try and fix this bug I found? Steps to reproduce
- Stop and delete your hugo docker container if running
- Pull in the latest of this branch
- Edit line 144 of the keycloak file to be as shown below.:
Why does [this link](/hosting/sso/keycloak/#copy-secret) break my brain?
- Start up the docker container:
docker compose up
Expected: The docs site compiles fine and web server starts on http://localhost:1313 and shows this log entry in the container:
cht-hugo | Web Server is available at http://localhost:1313/ (bind address 0.0.0.0)
cht-hugo | Press Ctrl+C to stop
Actual: The docs site never gets past Start building sites … and showing the hugo version:
cht-hugo | Start building sites …
cht-hugo | hugo v0.152.2
Note: if you use [this link](/hosting/sso/keycloak/) it works fine, but both [this link](/hosting/sso/keycloak/#copy-secret) and [this link](/hosting/sso/keycloak#copy-secret) cause the bug.
Maybe there's some recursive logic that it gets stuck in? I doubt this b/c CPU load is nominal when it fails to build.
We may ultimately choose to ship this as is and see how bad it gets, but it feels like a footgun we should try and avoid.
|
Also - lovely AI disclosure per our request and the offer to share logs is icing on the cake - thank you!! |
Thanks for the testing. I'll take a look at this. I may not get to it until tomorrow. In the interest of learning a little more, I'm curious how you knew to test that scenario. If you have time to share insights, they would be appreciated. |
|
No rush on getting back to this! Speaking of which, most of Medic will be off Dec 22 - Jan 2. otherwise it was total luck I found this issue! When I was in there fixing all the links, suddenly hugo stopped working (as described). I knew it was one of my commits in my PR and narrowed it down to one of them. Ultimately, it was Josh who bisected my commits to figure out it was the specific link I cited above that was causing the issue 🤷 |
|
I believe I've fixed the hang when pages link to themselves with fragments (e.g., The problem: Accessing The solution: Detect self-references by comparing permalinks. When found, use Result: Validation of links should now work, including self-referencing ones with full validation. Does this break anything else in validation?I don't think it will. It is contained in an if/else block that is scoped to pages with self-references. I tested some other link validations, and they worked okay. AI DisclosureI collaborated with Claude Code on this change. Before taking this approach, Claude Code suggested skipping fragment validation for self-referencing links. You can see that approach in 60ee295. If you prefer that approach, we can roll back to that commit. |
mrjones-plip
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work on a quick turn around!
I did a git pull origin on your branch and confirmed I'm on your latest commit:
git rev-parse --short HEAD
69a6547a
However, the issue still persists when I follow the steps to reproduce. Could you please take another look?
I took a video of my test steps in case I'm skipping something important. Let me know if I am!
Video showing test failing
Screencast.From.2025-12-11.13-36-12.mp4
This reverts commit 69a6547.
|
Interesting. FWICT, it only seems to hang when inside a tab block. In my testing earlier, I had added the test link outside of the tab, not inside of it. Today I have tested placing the test link in other places:
It hangs in both tab blocks but nowhere else. When I roll back to commit 60ee295, which skips fragment validation for self-referencing links, it also still hangs when the self-referencing link is in a tab block, but not in the other situations. I can continue to try to explore fixes. Or we can document this shortcoming somewhere. |
mrjones-plip
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the research! And wow - what luck that we found it then, as that's a very narrowly defined failure mode. Let's leave it as is (failing only inside a tab block) and then document this caveat.
Approving now knowing you'll add the note in the readme.
I'll get someone else to do the final review!
|
@witash - you feeling like diving into some docs trickery? Tagged you in for a review, but lemme know if it's not a fit and I can grab someone else! |
We probably want to remove my commits that tried to fix it, then. I'll revert the last two commits, verify that this still only happens in a tab block, and then add documentation. |
This reverts commit 60ee295.
…-checking-not-working-with-markdown-links
|
Thanks again for your help on this @cliftonmcintosh ! It looks like most folks are signing off for the holiday break. We might ship this week, but likely will wait 'til the start of the new year. |
andrablaj
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the massive work @cliftonmcintosh! ✨
Added two minor non-blocking comments about the README.
Co-authored-by: Andra Blaj <[email protected]>
|
@cliftonmcintosh - thanks so much for a great PR! I'm excited to be able to merge this before years end so we can start off the new year with a clean slate of all good links! Medic will be mostly closed until Jan 5th - see you then! |

Description
Implements automatic validation of internal markdown links at build time to prevent broken links from being deployed.
Changes
Added custom link render hook (layouts/_default/_markup/render-link.html)
linkErrorLevelconfiguration from hugo.tomlFixed Hextra theme breadcrumb compatibility (layouts/docs/list.html:8)
Added development workflow features
Fixed all broken internal links
Testing
linkErrorLevel = "ERROR"if broken links are presentlinkErrorLevel = "WARNING"andhighlightBroken = trueallows iterative fixing with visual feedbackCloses #2069
AI Disclosure
This PR was developed using Claude Code. Use included:
When working with an AI coding assistant, the assistant and I keep logs of our work. I can provide those documents if it would be useful to see how I work with the assistant.
License
The software is provided under AGPL-3.0. Contributions to this project are accepted under the same license.