From 522e17475f94751da2689ca04f4a75be2056914d Mon Sep 17 00:00:00 2001 From: Andy Stark Date: Wed, 12 Aug 2026 14:19:48 +0100 Subject: [PATCH] DOC-6909 Teach the feed pipeline and version archiver the /content/ link form Prepares the two tools that read relref as a literal string, ahead of any content moving off it. Both changes are inert until content actually uses repo-root-relative links, so this is safe to land on its own. The AI-facing Markdown and JSON output rewrote only relref to absolute redis.io URLs, so a migrated page would ship raw source paths that resolve nowhere. The version archiver had the same blind spot with worse consequences: links in an archived version would keep resolving to the latest page instead of the frozen copy, silently, with no error. Both patterns anchor on a Markdown link destination rather than the bare content prefix, or they would rewrite the github.com blob URLs that legitimately embed the same path segment. The archiver shares one replace_link closure between notations so the release-notes exemption and the already-versioned guard cannot drift apart. Source-relative links need no archiver rewriting: the whole subtree is copied, so a link between two pages inside it already resolves within the version. The accompanying tests pin that, and were confirmed to fail with only the new substitution disabled. Amended after Bugbot found that the first version of this commit silently reverted two DOC-6939 improvements to the same partial. The stack was carved out of the old single PR by checking each file out wholesale from a tag, which replaces the file rather than applying a delta, so any change main had made to it in the meantime was undone: the line-anchored HTML-comment strip, the table-children expansion, and a catch-all shortcode regex that had been tightened from [^>]* to a non-greedy match. Nothing warned, because reverting those produces silently wrong feed output rather than a build error, and the verification build was green. The rules are now inserted into main's current version of the file, so this commit is purely additive. Constraint: carve a branch out of an old state with a delta, never a wholesale file checkout from a tag, or any change main made to that file since the branch point is silently reverted Ticket: DOC-6909 Co-Authored-By: Claude Opus 5 --- build/test_version_archiver.py | 161 ++++++++++++++++++ build/version_archiver.py | 19 ++- .../partials/process-markdown-content.html | 12 ++ 3 files changed, 191 insertions(+), 1 deletion(-) create mode 100644 build/test_version_archiver.py diff --git a/build/test_version_archiver.py b/build/test_version_archiver.py new file mode 100644 index 0000000000..08ddeccf87 --- /dev/null +++ b/build/test_version_archiver.py @@ -0,0 +1,161 @@ +#!/usr/bin/env python3 +""" +Test script for version_archiver's link versioning. + +When a version is archived, intra-product links must point at the frozen copy, +not at latest. The archiver originally rewrote only `relref`, so a section +migrated to plain Markdown links (DOC-6909) was silently left pointing at +latest -- wrong content in an archived version, with no error or warning. + +These tests cover both notations, the guards they share, and the forms that must +NOT be touched. +""" + +import os +import sys +import tempfile + +# Add the build directory to the path +sys.path.insert(0, os.path.dirname(__file__)) + +from version_archiver import VersionArchiver + + +def archive(product, version, page_relpath, content): + """Run the real version_relrefs() over one page in an isolated tree. + + page_relpath is relative to the versioned directory, so nesting can be + realistic -- it matters for source-relative links, which resolve against the + page's own location. + """ + cwd = os.getcwd() + with tempfile.TemporaryDirectory() as tmp: + arch_cwd = tmp + os.chdir(arch_cwd) + try: + archiver = VersionArchiver(product, version) + page = os.path.join(archiver.new_directory, page_relpath) + os.makedirs(os.path.dirname(page), exist_ok=True) + with open(page, "w") as f: + f.write(content) + archiver.version_relrefs() + with open(page) as f: + return f.read() + finally: + os.chdir(cwd) + + +DEEP = os.path.join("databases", "configure", "page.md") + + +def test_relref_is_versioned(): + """The original behaviour: an intra-product relref gains the version.""" + out = archive("rs", "9.9", DEEP, + '[a]({{< relref "/operate/rs/databases/memory/eviction" >}})') + assert "/operate/rs/9.9/databases/memory/eviction" in out, out + print("✓ relref link is versioned") + + +def test_plain_content_link_is_versioned(): + """DOC-6909's repo-root-relative form must be versioned the same way.""" + out = archive("rs", "9.9", DEEP, + '[b](/content/operate/rs/databases/memory/eviction.md)') + assert "](/content/operate/rs/9.9/databases/memory/eviction.md)" in out, out + print("✓ plain /content/ link is versioned") + + +def test_plain_content_link_keeps_anchor(): + """An anchor must survive versioning.""" + out = archive("rs", "9.9", DEEP, + '[c](/content/operate/rs/databases/memory/eviction.md#policies)') + assert "/operate/rs/9.9/databases/memory/eviction.md#policies" in out, out + print("✓ anchor preserved when versioning a plain link") + + +def test_source_relative_link_is_left_alone(): + """Source-relative links need no rewriting and must not be touched. + + The whole subtree is copied, so a link between two pages inside it already + resolves within the versioned directory. + """ + link = '[d](../memory/eviction.md)' + out = archive("rs", "9.9", DEEP, link) + assert out == link, out + # and confirm the claim: it resolves inside the frozen tree + page_dir = os.path.join("content", "operate", "rs", "9.9", + os.path.dirname(DEEP)) + resolved = os.path.normpath(os.path.join(page_dir, "../memory/eviction.md")) + assert resolved.startswith(os.path.join("content", "operate", "rs", "9.9")), resolved + print("✓ source-relative link untouched, and resolves inside the version") + + +def test_release_notes_are_exempt(): + """Release notes are deliberately not versioned, in either notation.""" + both = ('[e]({{< relref "/operate/rs/release-notes/rs-7-8" >}})\n' + '[f](/content/operate/rs/release-notes/rs-7-8.md)') + out = archive("rs", "9.9", DEEP, both) + assert out == both, out + print("✓ release-notes links exempt in both notations") + + +def test_already_versioned_is_idempotent(): + """Re-running must not double-version an already-versioned link.""" + both = ('[g]({{< relref "/operate/rs/9.9/databases/memory/eviction" >}})\n' + '[h](/content/operate/rs/9.9/databases/memory/eviction.md)') + out = archive("rs", "9.9", DEEP, both) + assert out == both, out + assert "9.9/9.9" not in out, out + print("✓ already-versioned links are left alone (idempotent)") + + +def test_other_product_and_external_urls_untouched(): + """Only the product being archived is rewritten, and external URLs are safe. + + The GitHub blob URL is the important one: it contains the substring + '/content/operate/rs/', so the pattern must anchor on a link destination + ('](/content/...') rather than matching anywhere in the line. + """ + content = ('[i](/content/operate/kubernetes/deploy/quickstart.md)\n' + '[j](https://github.com/redis/docs/blob/main/content/operate/rs/x.md)\n' + '[k]({{< relref "/develop/data-types/hashes" >}})') + out = archive("rs", "9.9", DEEP, content) + assert out == content, out + print("✓ other products, external URLs and other sections untouched") + + +def test_other_products_use_their_own_prefix(): + """The pattern is parameterised, so non-'operate' products work too.""" + out = archive("redis-data-integration", "1.20", DEEP, + '[l](/content/integrate/redis-data-integration/reference/config.md)') + assert "/integrate/redis-data-integration/1.20/reference/config.md" in out, out + print("✓ redis-data-integration (integrate prefix) is versioned") + + +def main(): + tests = [ + test_relref_is_versioned, + test_plain_content_link_is_versioned, + test_plain_content_link_keeps_anchor, + test_source_relative_link_is_left_alone, + test_release_notes_are_exempt, + test_already_versioned_is_idempotent, + test_other_product_and_external_urls_untouched, + test_other_products_use_their_own_prefix, + ] + try: + for t in tests: + t() + print("\n✅ All tests passed!") + return 0 + except AssertionError as e: + print(f"\n❌ Test failed: {e}") + return 1 + except Exception as e: + print(f"\n❌ Unexpected error: {e}") + import traceback + traceback.print_exc() + return 1 + + +if __name__ == '__main__': + sys.exit(main()) diff --git a/build/version_archiver.py b/build/version_archiver.py index 219de52aa2..d2f1abb7f7 100644 --- a/build/version_archiver.py +++ b/build/version_archiver.py @@ -49,6 +49,19 @@ def update_relrefs(self, file_path, version, product): + re.escape(product) + r'/([^"]+)" ?>\}\})' ) + # Repo-root-relative Markdown links replace relref in sections migrated + # for DOC-6909, and need the same versioning. Without this they keep + # resolving to the latest page instead of the copy being frozen, which is + # silently wrong content in an archived version (no error, no warning). + # Source-relative links need no rewriting: the whole subtree is copied, so + # a link between two pages inside it already resolves within the version. + plain_pattern = ( + r'(\]\(/content/' + + self.prefix + + "/" + + re.escape(product) + + r'/([^)]+)\))' + ) with open(file_path, "r") as file: lines = file.readlines() @@ -74,8 +87,12 @@ def replace_link(match): return f"{new_link}" return full_match - # Replace all relref links in the line + # Replace all relref links in the line, then the plain Markdown ones. + # Both share replace_link: each match contains "///", + # so the same substitution and the same release-notes and + # already-versioned guards apply to either notation. modified_line = re.sub(pattern, replace_link, lines[i]) + modified_line = re.sub(plain_pattern, replace_link, modified_line) # If the line was modified, update the lines list if modified_line != lines[i]: diff --git a/layouts/partials/process-markdown-content.html b/layouts/partials/process-markdown-content.html index dd0a7297d9..1a0252d99e 100644 --- a/layouts/partials/process-markdown-content.html +++ b/layouts/partials/process-markdown-content.html @@ -60,6 +60,18 @@ {{- /* Pattern for literal: {{< relref "path" >}} - capture optional leading slash */ -}} {{- $content = $content | replaceRE `\{\{<\s*relref\s+"/?([^"]+)"\s*>\}\}` "https://redis.io/docs/latest/$1" -}} +{{- /* Fix repo-root-relative Markdown links (`](/content/...)`), which replace relref + in sections migrated for DOC-6909. Without this they reach the AI-facing + Markdown/JSON output as raw source paths, which resolve nowhere on the + published site. Rewritten to the same absolute form as relref above. + Section/leaf-bundle forms go first so the index segment is dropped rather + than left in the URL; `/index.md` mirrors the render-link hook, which + strips it, even though no content currently uses that form. A trailing + `#anchor` is outside each match and so survives untouched. */ -}} +{{- $content = $content | replaceRE `\]\(/content/([^)#]*?)/_index\.md` "](https://redis.io/docs/latest/$1" -}} +{{- $content = $content | replaceRE `\]\(/content/([^)#]*?)/index\.md` "](https://redis.io/docs/latest/$1" -}} +{{- $content = $content | replaceRE `\]\(/content/([^)#]*?)\.md` "](https://redis.io/docs/latest/$1" -}} + {{- /* Fix images - handle both HTML-escaped entities and literal characters */ -}} {{- /* Pattern for HTML-escaped: {{< image filename="path" [alt="..."] >}} */ -}} {{- $content = $content | replaceRE "\\{\\{<\\s*image\\s+filename="/?([^&]+)"[^}]*>\\}\\}" "![$1](https://redis.io/docs/latest/$1)" -}}