DOC-6939 Make the page-level id unique - #3761
Merged
Merged
Conversation
The record id was the content file's base name, so it was never unique: 5,733 pages shared 2,161 ids, with "message_history" naming 58 different pages and "install" 42. In the published feed that is 2,643 pages under 2,119 ids, so a consumer treating id as a primary key silently lost about 500 pages with nothing to notice it by. The applied AI team hit this and worked around it by keying on url; they have since confirmed they are content to keep keying on url provided the id is actually unique, which makes this an implementation rather than a decision. It is now the content path with the extension and any trailing /_index removed, so develop/clients/redis-py.md becomes develop/clients/redis-py. Unique by construction, because two files cannot share a path, and stable across builds because nothing about it depends on build order. A dedup suffix was rejected for that second reason. Appending -1 and -2 to collisions would have been a smaller change, but which page got the plain id and which got the suffix would depend on the order pages were processed, so an id could migrate between pages between builds and quietly repoint anyone who had stored one. An id that is not stable is barely better than an id that is not unique. Slashes are kept rather than flattened to hyphens, because flattening reintroduces a collision risk -- "a/b-c" and "a-b/c" both becoming "a-b-c" -- for no gain, and the path form matches how the url is already structured. The derivation lives in one partial rather than being written out in each template. It was previously duplicated three times, in single.json, section.json and section.json's children[] loop, and that pattern of the same rule implemented twice and then drifting has been the recurring theme of this whole ticket. children[].id in particular has to equal the child page's own id or the navigation graph does not resolve. One template gotcha, which the verification caught and reasoning would not have: writing the path normalisation as ".Path | replace \"\\\\\" \"/\"" makes .Path the LAST argument, so the backslash becomes the input string and every id on every page came out as a single backslash. The build succeeded and all 5,717 children[] entries still "resolved", because they all resolved to the same broken value. Only checking the actual ids showed it. 5,733 pages now carry 5,733 distinct ids with no collisions, all 5,717 children[] entries resolve to a real page id, and the section-id, example-id and content_hash invariants are unchanged. Learned: in a Hugo template, piping into replace puts the piped value LAST, so ".Path | replace old new" silently uses the pattern as the input -- every id became a single backslash, the build passed, and the children[] cross-reference check still showed 100% because everything resolved to the same wrong value Constraint: the page id must stay derivable from the content path alone, never from build order -- a dedup suffix would let an id migrate between pages between builds, which is worse than a duplicate because a stored reference silently repoints Constraint: single.json, section.json and its children[] loop must all take the id from layouts/partials/page-id.html, because children[].id has to equal the child page's own id for the navigation graph to resolve Rejected: suffixing colliding ids with -1 and -2 | smaller change, but assignment depends on processing order so ids are no longer stable across builds Rejected: flattening the path to hyphens for a more conventional-looking id | reintroduces collisions, since "a/b-c" and "a-b/c" both become "a-b-c" Ticket: DOC-6939 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
Contributor
🧠 Redis MemoryFound 5 related items from repository history:
Memory updated at f090ac6 |
Contributor
andy-stark-redis
added
infrastructure
AI-friendliness
Contributor
Author
|
Thanks @dwdougherty ! |
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
2 participants
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 the page-level
idunique — item A4 of DOC-6939.The defect
idwas the content file's base name, so it was never unique. 5,733 pages shared 2,161 ids:message_historynamed 58 different pages,install42,cli38. In the published feed that's 2,643 pages under 2,119 ids, so a consumer treatingidas a primary key silently lost about 500 pages, with nothing to notice it by.The applied AI team hit this and worked around it by keying on
url. They've since confirmed they're happy to keep keying onurlprovided theidis genuinely unique — which turns this from a decision into an implementation.The fix
idis now the content path with the extension and any trailing/_indexremoved:content/commands/set.mdcommands/setcontent/develop/clients/redis-py.mddevelop/clients/redis-pycontent/develop/clients/_index.mddevelop/clientsUnique by construction — two files can't share a path — and stable across builds, because nothing about it depends on processing order.
children[]entries resolving to a page idSection ids, example ids and
content_hashare unaffected.Two alternatives rejected
Dedup suffixes (
-1,-2) would have been a smaller change, but which page gets the plain id and which gets the suffix depends on processing order — so an id could migrate between pages between builds and silently repoint anyone who had stored one. An unstable id is worse than a duplicate: a duplicate fails loudly the first time you compare counts, a migrating id doesn't fail at all.Flattening slashes to hyphens, for a more conventional-looking id, reintroduces collisions —
a/b-canda-b/cboth becomea-b-c.Note on structure
The derivation now lives in one partial,
layouts/partials/page-id.html. It was previously written out three times —single.json,section.json, andsection.json'schildren[]loop — and "same rule implemented twice, then drifted" has been the recurring theme of this ticket.children[].idin particular must equal the child page's own id or the navigation graph doesn't resolve.What a reviewer should focus on
idvalues change for every page, not only colliding ones. It's a consumer-visible field, though the one known consumer keys onurland asked for this.idis updated to match. That line also gets touched by DOC-6939 Document feed coverage and the section role vocabulary #3759, in a different hunk.One gotcha, recorded because the tests didn't catch it
I first wrote the path normalisation as
.Path | replace "\\" "/". In Hugo, piping puts the piped value last, so that evaluates asreplace "\\" "/" .Path— the backslash became the input and every id came out as a single backslash. The build passed, and thechildren[]cross-reference check reported 5,717/5,717 resolving, because everything resolved to the same wrong value. Only printing actual ids caught it. Worth knowing that a cross-reference check can be fully satisfied by uniformly wrong data.🤖 Generated with Claude Code
Note
Medium Risk
Consumer-visible breaking change to
idon all documentation JSON records; correctness improves but any client still keying on the old basename ids must migrate (known consumer usesurl).Overview
Fixes duplicate
idvalues in AI/RAG JSON output by replacing filename-based ids with the page’s content path (extension and trailing/_indexstripped), e.g.develop/clients/redis-py.Adds shared partial
layouts/partials/page-id.htmland wiressingle.json,section.json, andchildren[].idthrough it so section navigation ids match each child page’s own record. Pages without a backing file still use a urlized title fallback.Updates
content/ai-agent-resources.mdso the documentedidfield matches the new semantics. Every page’sidchanges in the published feed—not only previously colliding ones.Reviewed by Cursor Bugbot for commit f090ac6. Bugbot is set up for automated code reviews on this repo. Configure here.