docs: correct the contributor-facing record - #196
Conversation
Every human-readable doc said Node 20; every machine-readable file said 22. A contributor following the docs installs a version on which `npm run lint` dies before linting anything, because eslint-plugin-unicorn evaluates Set.prototype.union at module load. Corrected in README, GettingStarted, CLI, CLI-Quickstart and AGENTS.md, with the reason stated so the next person does not helpfully lower it again. Two CI steps were also named "Use Node.js 20" while installing 22. Adds CONTRIBUTING.md, which did not exist - GitHub's contributing link was dead and the only substitute was two bullets pointing at an agent-memory file. It carries the Node floor and why, the .npmrc legacy-peer-deps requirement (previously explained only inside .npmrc itself, so anyone "cleaning it up" breaks npm ci), the two-origin dev topology, how to run each suite, and the contract-suite rule for anything with two adapters. docs/LocalDB.md rendered as one giant code block on GitHub: a five-backtick fence opened at line 14 and nothing valid closed it until the last line, so the whole document - the only place the web app dev flow is written down - was unreadable. Every fence is now balanced and closes without an info string, which CommonMark does not permit. Content is byte-identical apart from fences. .env.example listed three SUPABASE_* variables that no code in this repository reads, while omitting most that it does. Replaced with the real set, defaults included. GettingStarted gains a web-app section explaining that the API and the UI are separate origins and that DB8_ALLOWED_ORIGINS governs the split - previously documented only in docs/Ops.md, which nothing linked to. docs/README.md now maps the guides, design docs and process docs, not only the specs; README links it and Ops from the front door. db8-readme.md, a stale duplicate whose quickstart calls a `dev:server` script that does not exist, is marked historical rather than deleted. CHANGELOG gained the two missing entries - the Node 22 / eslint 10 toolchain change, and the VerdictStore port including the memory-mode capacity refusal, which is a behaviour change that shipped unlogged - and the browser claim term editor is folded into the day it landed. One code change, because the docs could not be truthful without it: SUBMIT_WINDOW_SEC had two defaults, 300 via config-builder and 3600 read straight from process.env in RoomService. RoomService now reads the injected config, so the declared default is the only default.
|
Warning Review limit reached
Next review available in: 50 minutes Limit details: You’ve used all 1 included review currently available under your plan. 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: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (16)
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 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 64327e8482
ℹ️ 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".
.claude/ holds the cool-ideas log and the code-smell journal. They are working notes rather than project documentation, and the convention keeps them out of the repository.
…racy # Conflicts: # .gitignore # CHANGELOG.md
…watcher's canon mode Nine review findings. Four were errors in documentation I wrote this week. - DB8_ANON is a directory name under db8/round-N/ for local drafts. I described it as "submit anonymously", which it is not. - CONTINUE_WINDOW_SEC defaults to 30, not 300. - ENFORCE_RATELIMIT is compared against the literal '1' at runtime (server/mw/rate-limit.js), so documenting `false` implied `true` would enable it. It would not. - CONTRIBUTING said `npm test` runs "once in memory mode and once with DB8_TEST_PG=1". Both passes have a database. The script's own labels say "fresh database" and "same database, DB-gated (idempotency gate)": the second pass re-runs over the state the first left behind, so a test that only passes on a pristine database fails there. That is a more useful thing to know than what I had written. - CONTRIBUTING also said no workflow runs the browser tests. The browser-tests job landed in #193. One code fix. server/watcher.js read `process.env.CANON_MODE || 'jcs'` and handed it straight to createSigner, so a typo silently signed journals as jcs while the operator believed they were sorted - the same silent-fallback bug just fixed in the CLI. normalizeCanonMode now validates it, and resolveCanonicalizer delegates to it so there is one definition of what a mode means rather than three. The CHANGELOG entry for the VerdictStore port was dated 2026-08-12, when the branch work started; #192 merged on the 15th. Redated and reordered. The GettingStarted CORS guidance the review called nonexistent now exists - #193 merged server/cors.js into this branch's base.
Activity Summary
Four of these were errors in documentation I wrote this week, which is the uncomfortable part. #5 taught me something. I wrote that #1 is a code fix, and the same bug I fixed in the CLI two PRs ago. #9 resolved itself: Verification339 passing, 0 failing. |
An audit of everything a new contributor reads. Findings ranked by "would this block or mislead someone".
Node 20 vs 22
Every machine-readable file said 22 —
.nvmrc,engines,docker-compose.test.yml, all three workflows. Every human-readable file said 20:README.md(which cited.nvmrc, contradicting it),GettingStarted,CLI,CLI-Quickstart,AGENTS.md×2. Two CI steps were even named "Use Node.js 20" while installing 22.Someone following the docs installs Node 20, and
npm run lintdies with aTypeErrorbefore linting anything, becauseeslint-plugin-unicornevaluatesSet.prototype.unionat module load.enginesis advisory and there is noengine-strict, so install succeeds and the failure arrives later, detached from its cause.Corrected everywhere, with the reason stated — otherwise the next person helpfully lowers it back.
There was no CONTRIBUTING.md
Not in the root, not in
.github/. GitHub's contributing link was dead, and the only substitute was two README bullets pointing atAGENTS.md, which is an agent-memory file.The new one carries: the Node floor and why; the
.npmrclegacy-peer-depsrequirement; the two-origin dev topology; how to run each suite (including thatnpm testruns twice, once per persistence mode); the Playwright suite and why it is excluded fromnpm test; and the rule that anything with two adapters gets its assertion in the shared contract suite..npmrc'slegacy-peer-deps=truewas explained only inside.npmrc. Anyone tidying it up breaksnpm ciin CI.docs/LocalDB.md was unreadable on GitHub
A five-backtick fence opened at line 14 and nothing valid closed it until the last line — so the entire document rendered as one code block. No headings, no links. That file is the only place the web app dev flow is written down.
Closing fences also carried
textinfo strings, which CommonMark does not permit. Every fence is now balanced; content is byte-identical apart from fence lines (verified by diffing with fences stripped)..env.examplewas fictionIt listed
SUPABASE_URL,SUPABASE_ANON_KEY,SUPABASE_SERVICE_ROLE_KEY— read by no code in this repository — while omitting most variables that are.AGENTS.mdtells contributors to copy it and "fill values as needed", sending them after three secrets that do nothing. Replaced with the real set and their defaults.The web app was undocumented
GettingStartednever mentionedweb/at all. It now explains that the API and the UI are separate origins and thatDB8_ALLOWED_ORIGINSgoverns it — previously documented only indocs/Ops.md, which nothing in the repo linked to.docs/README.mdmapped only the specs; it now maps guides, design and process docs, andREADME.mdlinks it and Ops from the front door.db8-readme.md— a stale duplicate whose quickstart calls adev:serverscript that does not exist — is marked historical rather than deleted.CHANGELOG omissions
Two changes shipped unlogged: the Node 22 / eslint 10 toolchain change (the most contributor-affecting change of the last twenty commits), and the VerdictStore port, including memory-mode verdict writes now being refused at a capacity bound — a behaviour change. The browser claim term editor is folded into the day it landed.
One code change
The docs could not be truthful without it.
SUBMIT_WINDOW_SEChad two defaults: 300 viaconfig-builder, and 3600 read straight fromprocess.envinRoomService. Documenting either would be wrong for the other path.RoomServicenow reads the injected config, so the declared default is the only default.Verification
308 passing, 0 failing.
eslint,markdownlint,cspell,prettierclean; every link in the new documentation map resolves.