docs: correct the repo docs against what the code actually does - #1478
Conversation
An audit read every markdown file against the tree it describes. This applies what it found, in three groups. Wrong, where a reader following the docs would be misled: the Node and pnpm versions, `getSdkAuthContext`'s arity, the private-API host resolution, `cp .env.template` from a path that does not exist, Hivesigner variables named with a NEXT_PUBLIC_ prefix that would ship a server secret to the browser, compose paths, and `master`/`development` branch names that have been `main` and `develop` for a while. Two cache claims were confidently backwards: most tiers ARE cached for logged-in users (only the mute-filtered feeds go private), and the post-age cold miss is a 60s `entry-unknown` tier rather than an hour. Incomplete, where the shape has moved on: `pnpm test` is web-only and `pnpm build` is not recursive, so both names oversell what they run; `pnpm typecheck` does not reach hosting/api at all; production deploys BOTH regions; the tier table was missing two tiers; the hosting diagram showed a service that does not exist and omitted Redis; Model B was described as an option rather than as the alternative that was rejected. Missing, where a whole subsystem had no prose: the theme platform and its component seams, the claim landing and its runtime-gated noindex, the signup funnel that customizes before payment, the free bundle path that creates no tenant, `POST /v1/tools/compose-config`, the SEO writer, per-post SSI metadata, the Configuration Editor, and how to cut a release. `apps/self-hosted` was not mentioned in the root README at all. Counts that drift every merge are dropped rather than re-pinned. Two source comments are corrected in the same pass, both stale in ways that would mislead: registry.ts still said all five templates are CSS-only, and the roster's add-a-template checklist omitted the display catalog, which is satisfies-checked and so fails typecheck on a missing id. Depends on #1475: the layout options it retires are already absent here.
|
Warning Review limit reached
Next review available in: 70 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (9)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Warning Review limit reached
Next review available in: 96 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (9)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
PR Summary by QodoDocs: align repository documentation with current code and deployment flows
AI Description
Diagram
High-Level Assessment
Files changed (9)
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 01dd8be3be
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| - ✅ Configurable sidebar and branding | ||
| - ✅ Claim landing: an unclaimed subdomain serves the shared template config and renders a claim CTA plus a read-only live preview, instead of a blog | ||
|
|
||
| **The theme platform.** `src/themes/manifest.ts` defines a `ThemeComponents` seam (Shell, Navigation, Sidebar, ArchiveList, PostCard); a template's manifest in `src/themes/registry.ts` overrides the seams it owns and everything absent falls back to the shared default, so a CSS-only template resolves to the very same component functions. A manifest also declares `unsupportedOptions` (today `['sidebar']`), which the Configuration Editor uses to HIDE the control rather than leave a toggle that does nothing. |
There was a problem hiding this comment.
The documented value of unsupportedOptions does not match the theme manifests: Journal, Reader, Gallery, and Terminal all declare ['sidebar', 'listType'], and ThemeOptionKey includes both values. A contributor relying on this architecture guide could omit the required listType declaration when adding a structural theme, contrary to the manifest contract described elsewhere; either list both values or clarify that only sidebar currently corresponds to a visible editor control.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Both of you flagged this line, and the mismatch was real on the commit you reviewed, but the cause is the branch base rather than the sentence. Resolved in d24eae1.
This branch was cut from df47c67, and #1475 (which retires listType) merged afterwards. So at the head you reviewed, the code genuinely said one thing and the docs another:
reviewed head 01dd8be |
develop today | |
|---|---|---|
ThemeOptionKey |
'sidebar' | 'listType' |
'sidebar' |
| journal / reader / gallery / terminal | ['sidebar', 'listType'] |
['sidebar'] |
The PR description marked it "merge after #1475" for exactly this reason. #1475 is now merged and deployed, so I merged develop in rather than editing the sentence: unsupportedOptions really is ['sidebar'] today, and widening the docs to re-include listType would have reintroduced the drift in the opposite direction, documenting an option that no longer exists.
Worth saying plainly, though: the underlying check was the right one to make. A docs PR is exactly where a stale base produces a claim that is wrong at merge time, and neither of you could see the pending PR that resolved it.
It also made me re-read my own neighbouring text, where I found a genuine version of the same error and fixed it in the same commit. The registry comment said each of the five structural themes "declares what it cannot honour in unsupportedOptions". That is true of four. Magazine overrides ArchiveList but keeps the shared shell, so its sidebar still works and it declares nothing. Owning a seam and dropping an option are separate things, and the comment now says so.
Code Review by Qodo🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)
Great, no issues found!Qodo reviewed your code and found no material issues that require reviewTip of the day💡 Did you know, you can type 'qodo, fix this' on a finding and the fix lands right on your PR |
Code Review by Qodo
1. Wrong unsupportedOptions docs
|
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
| - ✅ Configurable sidebar and branding | ||
| - ✅ Claim landing: an unclaimed subdomain serves the shared template config and renders a claim CTA plus a read-only live preview, instead of a blog | ||
|
|
||
| **The theme platform.** `src/themes/manifest.ts` defines a `ThemeComponents` seam (Shell, Navigation, Sidebar, ArchiveList, PostCard); a template's manifest in `src/themes/registry.ts` overrides the seams it owns and everything absent falls back to the shared default, so a CSS-only template resolves to the very same component functions. A manifest also declares `unsupportedOptions` (today `['sidebar']`), which the Configuration Editor uses to HIDE the control rather than leave a toggle that does nothing. |
There was a problem hiding this comment.
1. Wrong unsupportedoptions docs 🐞 Bug ≡ Correctness
CLAUDE.md now states manifests declare unsupportedOptions as “today ['sidebar']”, but the theme manifest type and registry include listType as a supported/used unsupported option as well. This can mislead contributors working on template support or Configuration Editor gating.
Agent Prompt
## Issue description
`CLAUDE.md` claims `unsupportedOptions` is currently `['sidebar']`, but the self-hosted theme manifest contract supports both `sidebar` and `listType`, and several manifests use both. This is a docs-vs-code mismatch.
## Issue Context
The new CLAUDE.md theme-platform paragraph is intended to be an accurate guide for extending templates and the Configuration Editor.
## Fix Focus Areas
- CLAUDE.md[239-243]
## Proposed fix
Update the parenthetical to reflect reality (e.g. “(e.g. `['sidebar', 'listType']` depending on template)” or remove the “today ...” claim and describe the mechanism generically: `unsupportedOptions` can include `sidebar` and/or `listType`).
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
There was a problem hiding this comment.
Both of you flagged this line, and the mismatch was real on the commit you reviewed, but the cause is the branch base rather than the sentence. Resolved in d24eae1.
This branch was cut from df47c67, and #1475 (which retires listType) merged afterwards. So at the head you reviewed, the code genuinely said one thing and the docs another:
reviewed head 01dd8be |
develop today | |
|---|---|---|
ThemeOptionKey |
'sidebar' | 'listType' |
'sidebar' |
| journal / reader / gallery / terminal | ['sidebar', 'listType'] |
['sidebar'] |
The PR description marked it "merge after #1475" for exactly this reason. #1475 is now merged and deployed, so I merged develop in rather than editing the sentence: unsupportedOptions really is ['sidebar'] today, and widening the docs to re-include listType would have reintroduced the drift in the opposite direction, documenting an option that no longer exists.
Worth saying plainly, though: the underlying check was the right one to make. A docs PR is exactly where a stale base produces a claim that is wrong at merge time, and neither of you could see the pending PR that resolved it.
It also made me re-read my own neighbouring text, where I found a genuine version of the same error and fixed it in the same commit. The registry comment said each of the five structural themes "declares what it cannot honour in unsupportedOptions". That is true of four. Magazine overrides ArchiveList but keeps the shared shell, so its sidebar still works and it declares nothing. Owning a seam and dropping an option are separate things, and the comment now says so.
#1475 has landed, so the retired layout options are now absent from the code this branch documents.
…comment Magazine overrides ArchiveList but keeps the shared shell, so its sidebar still works and it declares no unsupportedOptions. Saying every structural theme declares one was the same class of docs-vs-code drift this branch exists to fix.
1 participant
Docs only, plus two stale source comments. No behaviour changes.
An audit read every markdown file in the repo against the code it describes, and verified each finding against a cited file rather than against another document. This applies the result.
Wrong, where following the docs would mislead you
node ^18.17.xandpnpm 10.18.1whenenginessays>=22.12andpackageManagersays11.5.0getSdkAuthContext(activeUser, activeUser?.username)in two snippets, against a one-parameter signaturecp .env.template .envfrom the repo root, where no such file existsNEXT_PUBLIC_HS_CLIENT_ID/NEXT_PUBLIC_HS_CLIENT_SECRET, which appear nowhere in the source. The real names areHIVESIGNER_CLIENT_IDandHIVESIGNER_SECRET, both server-only. Documenting aNEXT_PUBLIC_prefix on a shared secret invites shipping it to the browser.docker stack deploy -c docker-compose.ymlfrom a root that has no compose filemasteranddevelopmentbranches, which aremainanddevelopfeed,feed-createdandprofile-feedgo private; everything else is auth-class-equivalent and shared), and a "conservative 1h tier" on a post-age miss that is really a 60sentry-unknownIncomplete, where the shape has moved on
pnpm testis web-only andpnpm buildis not recursive, so both names oversell what they run.pnpm typechecknever reacheshosting/api, which sits outside the workspace globs. Production deploys both regions, not one. The tier table was missingdynamic-pageandentry-unknown. The hosting diagram showed a "Domain Verify Service" that does not exist and omitted Redis. Model B read as an available deployment option when nothing implements it.Missing, where a subsystem had no prose at all
The theme platform and its component seams, the claim landing, the signup funnel that customizes before payment, the free bundle path that creates no tenant,
POST /v1/tools/compose-config, the static SEO writer, per-post SSI metadata, the Configuration Editor and how to cut a release.apps/self-hosted, an entire second application, was not mentioned in the root README.Two of these are worth calling out because the docs actively contradicted the code:
mainpublishes:latest. It does not.:latestmoves only on aself-hosted-vX.Y.Ztag, which is exactly what the tag gating in Self-hosted: coherent versioning across images, configs and templates #1460 was built to guarantee, and cuttingself-hosted-v1.0.0yesterday confirmed it in the wild.noindexis applied at RUNTIME and gated on thetemplateflag, not written statically. That is deliberate, since a static noindex would deindex a live blog during any window where its config was missing, and nothing recorded the reason.Counts
Directory counts had all drifted (25 features against 33 on disk, 64 shared against 101, and so on). They are dropped rather than re-pinned, since they go stale on the next merge either way.
Source comments
Two comments were stale in ways that would mislead someone extending the code, so they are corrected here rather than left for the next reader:
registry.tsstill said "All five existing templates are CSS-only, so none carries a components key". Nine templates exist and five carry one.style-template-display.tsissatisfies Record<StyleTemplate, StyleTemplateDisplay>, so it fails typecheck until the new id has a card.Merge order
After #1475. The layout options that PR retires are already absent from this text, and it also takes the
listTypeline out of DEPLOYMENT.md's layout example, which would otherwise conflict.Every claim was checked against a cited file. Three items in the audit turned out to be wrong on re-verification and were corrected while applying rather than copied through: the compose stack has four services and not three, the add-a-template steps are four and not three, and Magazine overrides a seam without declaring the sidebar unsupported, so it keeps its sidebar.