DOC-6951 Publish redirects so a moved page is not read as a deletion - #3771
Open
andy-stark-redis wants to merge 2 commits into
Open
DOC-6951 Publish redirects so a moved page is not read as a deletion#3771andy-stark-redis wants to merge 2 commits into
andy-stark-redis wants to merge 2 commits into
Conversation
Hugo emits alias stubs for the HTML output format only. So a moved page's old URL serves a 200 meta-refresh page while its /index.json and /index.html.md both 404 -- verified on three correctly-aliased moves and reproduced against Hugo 0.143.1. Our own documentation tells consumers to find a page's JSON by appending /index.json to its URL, so following that instruction on a page that moved returns 404 and the move is indistinguishable from a deletion. That happens for every move, including the ones we alias correctly, which is why the alias backfill did not fix it and this does. Two mechanisms. At the old URL there is now a record with page_type "moved" carrying url and moved_to and nothing else, written after the build by walking the map Hugo renders rather than by parsing stub HTML. And /redirects.json publishes the whole map once, rendered from .Aliases -- the same data Hugo uses for the stubs, so the map and the site's real behavior cannot drift and there is no generated file to keep in step. The map's design changed once measurement contradicted the obvious version. Emitting one entry per declaration produced duplicate keys, 29 of them naming different targets, because two pages can declare the same alias. Hugo settles that by writing one stub and picking a winner arbitrarily, so publishing either target would have handed consumers an answer the site does not honour. Keys with exactly one target now go in `redirects` and contested ones in `ambiguous` with every candidate listed. 791 and 29 respectively in a full local build. Tombstones are only written for unambiguous keys, for the same reason. Tombstones stay out of docs.ndjson. They share the index.json name, so the feed's rglob would have swept in about a thousand pointer records against 2,600 real ones and made any count of the corpus ambiguous -- the exact confusion DOC-6939 spent its time reconciling. generate_ndjson.py filters on page_type instead. Verified: the record count is identical before and after the tombstone pass, and no moved record reaches the feed. page_type "moved" is a new value and moved_to a new field, so aiSchemaVersion goes to 2 by the definition the applied AI team proposed and we adopted. Refusing to bump on the first real change would have taught them to ignore the field. The page records also gain `aliases`, declared on both transform interfaces rather than left to ride on a spread, per the constraint from the schema_version work. It does not disturb content_hash, which covers summary, sections and examples only. Verified end to end on a full build and the real pipeline order: 774 tombstones from 791 map entries with 8 skipped for having no stub and 9 for already holding a real record, no duplicate keys in the map, the feed unchanged at 5,733 records, and a re-run of the transform skipping all 6,507 files rather than rewriting a tombstone. Learned: the naive map published contradictory data -- 29 keys with two targets each -- and only counting the keys showed it; the fix is to publish the contradiction rather than resolve it, because the site resolves it arbitrarily and any single answer would sometimes be wrong Constraint: tombstones are written only for unambiguous keys, never over an existing index.json, and only where Hugo actually emitted a stub -- an alias on a draft or on a taken URL gets nothing, matching the site Constraint: redirect tombstones must stay out of docs.ndjson, which is one record per documentation page; generate_ndjson.py filters page_type "moved" Gaps: the published base_url depends on CI rewriting baseURL in config.toml with sed, which still matches; not observed against a deployed build Ticket: DOC-6951 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
Contributor
🧠 Redis MemoryFound 5 related items from repository history:
Memory updated at 964d6b4 |
Contributor
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Two Bugbot findings, both correct, and both cases where the pass only worked because CI happens to build into a fresh tree. The no-overwrite guard was too broad. It existed to protect a real page's record, which it must, but it also refused to rewrite the pass's own tombstones -- so on an incremental build a tombstone kept a moved_to that had since changed. The guard now asks whether the file it found is one of ours, by reading page_type, and refuses only for anything else. Nothing removed a tombstone either. An alias that leaves the map, or becomes ambiguous and so moves into the candidate list, left its tombstone behind pointing somewhere the site no longer redirects. The pass now records the set it expects and sweeps any recognisable tombstone the map no longer names. Both are invisible in production, because every CI build starts from an empty directory. That is exactly why they were worth fixing: the script was correct by accident of its environment rather than by construction, and the next person to run it locally would have got a wrong answer with no signal. Verified against a real built tree: a re-run refreshes all 774 rather than skipping them, a deliberately poisoned moved_to is corrected, a planted orphan is removed, a real page record is byte-identical afterwards, and the feed still holds 5,733 records with no moved record leaking in. Learned: "correct because CI starts clean" is not correct -- both defects here were invisible in production and would have produced silently wrong output for anyone running the script twice Constraint: the guard protects real page records, not the pass's own tombstones; those are rewritten every run, and ones the map no longer names are deleted Ticket: DOC-6951 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
🧠 Redis MemoryFound 5 related items from repository history:
Memory updated at 32d3f1a |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 32d3f1a. Configure here.
| {{- $ambiguous := slice -}} | ||
| {{- range $from, $targets := ($seen.Get "map" | default dict) -}} | ||
| {{- if eq (len $targets) 1 -}} | ||
| {{- $redirects = $redirects | append (dict "from" $from "to" (index $targets 0)) -}} |
There was a problem hiding this comment.
Map lists unhonored alias routes
Medium Severity
redirects.json is built from every declared alias with a single target, but write_redirect_tombstones.py only writes tombstones where Hugo left an alias stub and skips paths with no index.html or with a real index.json. Those unambiguous redirects entries can still appear in the map while the old URL serves a live page or has no JSON, so bulk consumers can get a to URL that does not match from/index.json or the HTML route.
Reviewed by Cursor Bugbot for commit 32d3f1a. Configure here.
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
1 participant
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.
Makes a moved page resolvable instead of looking deleted — item D of DOC-6951. Based on
main, independent of #3767/#3769/#3770.The problem
Hugo emits alias stubs for the HTML output format only. So a moved page's old URL serves a 200 meta-refresh page, while its
/index.jsonand/index.html.mdboth 404.Our own
ai-agent-resourcespage tells consumers to find a page's JSON by appending/index.jsonto its URL. Following that instruction on a page that moved returns 404 — so a move is indistinguishable from a deletion, for every move, including the ~90% we now alias correctly. #3769 fixed alias coverage; it could not fix this.Two mechanisms
A record at the old URL.
page_type: "moved", carryingurlandmoved_toand nothing else:{"schema_version": 2, "page_type": "moved", "id": "data-types/bitmaps", "url": "https://redis.io/docs/latest/data-types/bitmaps/", "moved_to": "https://redis.io/docs/latest/develop/data-types/strings/bitmaps/"}(That's a real one from the build — and one of the two moves the applied AI team's assessment originally cited.)
A map of all of them at
/redirects.json, rendered from.Aliases— the same data Hugo uses for its own stubs, so map and site cannot drift and there's no generated file to keep in step.The map's design changed once I counted the keys
The obvious version — one entry per declaration — published duplicate keys, 29 of them naming different targets, because two pages can declare the same alias. Hugo settles that by writing one stub and picking a winner arbitrarily, so publishing either target would hand consumers an answer the site doesn't honour.
So
redirectsholds only keys with exactly one target (791), and contested keys go inambiguouswith every candidate listed (29). Tombstones are written only for unambiguous keys, for the same reason. That's the same report-don't-guess principle as the scanner's collision bucket in #3767 — this is the fourth place the duplicate-alias problem has surfaced.Tombstones stay out of
docs.ndjsonThey share the
index.jsonname, so the feed'srglobwould have swept in ~1,000 pointer records against 2,600 real ones, making any count of the corpus ambiguous — precisely the confusion DOC-6939 spent its time reconciling.generate_ndjson.pyfilters onpage_typeinstead.schema_version→ 2page_type: "moved"is a new value andmoved_toa new field, so this is a shape change by the definition the AI team proposed and we adopted. Refusing to bump on the first real change would teach them the field is inert.Page records also gain
aliases, declared on both transform interfaces rather than riding on a spread, per the constraint from theschema_versionwork. It doesn't disturbcontent_hash, which coverssummary,sectionsandexamplesonly.Verification — full build, real pipeline order
redirectsmovedrecords leaking into the feedThe no-overwrite guard did real work: 9 live pages left alone.
Review round (
32d3f1a5b)Two Bugbot findings, both correct, and both cases where the pass worked only because CI builds into a fresh tree:
page_typeand refuses only for a real page's record, rewriting its own tombstones every runBoth are invisible in production, which is precisely why they were worth fixing: the script was correct by accident of its environment rather than by construction, and anyone running it locally twice would have got a wrong answer with no signal.
Verified against a real built tree: a re-run refreshes all 774 rather than skipping, a deliberately poisoned
moved_tois corrected, a planted orphan is removed, a real page record is byte-identical afterwards, and the feed still holds 5,733 records with nothing leaking in.Known gap
The published
base_urldepends on CI rewritingbaseURLinconfig.tomlwithsed. That sed still matches after my edits — checked — but I haven't observed it against a deployed build.Not included
D3 —
data/page-moves.jsonfor move dates and deleted pages, the two things Hugo cannot know. Deferred; say the word.🤖 Generated with Claude Code