Skip to content

docs: correct the contributor-facing record - #196

Merged
flyingrobots merged 4 commits into
mainfrom
docs/contributor-accuracy
Aug 16, 2026
Merged

docs: correct the contributor-facing record#196
flyingrobots merged 4 commits into
mainfrom
docs/contributor-accuracy

Conversation

@flyingrobots

Copy link
Copy Markdown
Owner

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 lint dies with a TypeError before linting anything, because eslint-plugin-unicorn evaluates Set.prototype.union at module load. engines is advisory and there is no engine-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 at AGENTS.md, which is an agent-memory file.

The new one carries: the Node floor and why; the .npmrc legacy-peer-deps requirement; the two-origin dev topology; how to run each suite (including that npm test runs twice, once per persistence mode); the Playwright suite and why it is excluded from npm test; and the rule that anything with two adapters gets its assertion in the shared contract suite.

.npmrc's legacy-peer-deps=true was explained only inside .npmrc. Anyone tidying it up breaks npm ci in 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 text info 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.example was fiction

It listed SUPABASE_URL, SUPABASE_ANON_KEY, SUPABASE_SERVICE_ROLE_KEYread by no code in this repository — while omitting most variables that are. AGENTS.md tells 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

GettingStarted never mentioned web/ at all. It now explains that the API and the UI are separate origins and that DB8_ALLOWED_ORIGINS governs it — previously documented only in docs/Ops.md, which nothing in the repo linked to.

docs/README.md mapped only the specs; it now maps guides, design and process docs, and README.md 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 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_SEC had two defaults: 300 via config-builder, and 3600 read straight from process.env in RoomService. Documenting either would be wrong for the other path. RoomService now reads the injected config, so the declared default is the only default.

Verification

308 passing, 0 failing. eslint, markdownlint, cspell, prettier clean; every link in the new documentation map resolves.

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.
@coderabbitai

coderabbitai Bot commented Aug 15, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@flyingrobots, you've reached your PR review limit, so we couldn't start this review.

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 @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: e2d70103-fdfe-4997-aa13-5974a65bf026

📥 Commits

Reviewing files that changed from the base of the PR and between 7e008d2 and c34f294.

📒 Files selected for processing (16)
  • .env.example
  • .github/workflows/build-test.yml
  • AGENTS.md
  • CHANGELOG.md
  • CONTRIBUTING.md
  • README.md
  • db8-readme.md
  • docs/CLI-Quickstart.md
  • docs/CLI.md
  • docs/GettingStarted.md
  • docs/LocalDB.md
  • docs/README.md
  • server/canon-mode.js
  • server/services/RoomService.js
  • server/test/canon.mode.validation.test.js
  • server/watcher.js

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread docs/GettingStarted.md
Comment thread README.md
Comment thread CONTRIBUTING.md Outdated
Comment thread .env.example Outdated
Comment thread .env.example Outdated
Comment thread CONTRIBUTING.md
Comment thread CHANGELOG.md
Comment thread .env.example
Comment thread .env.example Outdated
.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.
@flyingrobots

Copy link
Copy Markdown
Owner Author

Activity Summary

# Sev Source File Issue Commit Outcome
1 P1 codex server/watcher.js Read CANON_MODE unvalidated and handed it to createSigner c34f294 Fixed
2 P1 codex .env.example DB8_ANON described as "submit anonymously"; it is a directory name c34f294 Fixed
3 P2 codex .env.example CONTINUE_WINDOW_SEC documented as 300; the default is 30 c34f294 Fixed
4 P2 codex .env.example ENFORCE_RATELIMIT=false implied true enables it c34f294 Fixed
5 P2 codex CONTRIBUTING.md Described the two test passes inaccurately c34f294 Fixed
6 P2 codex CONTRIBUTING.md Said no workflow runs the browser tests c34f294 Fixed
7 P2 codex CHANGELOG.md VerdictStore entry dated when the work started, not when it landed c34f294 Fixed
8 P1 codex README.md Stale lastUpdated after substantive edits c34f294 Fixed
9 P1 codex docs/GettingStarted.md CORS guidance referenced something that did not exist Resolved by merge

Four of these were errors in documentation I wrote this week, which is the uncomfortable part. ENFORCE_RATELIMIT is the sharpest: the middleware compares against the literal '1', so my false example implied true would work. It would not.

#5 taught me something. I wrote that 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 against a pristine database fails there. That is a far more useful thing for a contributor to know, and I had papered over it.

#1 is a code fix, and the same bug I fixed in the CLI two PRs ago. 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. normalizeCanonMode validates it now, and resolveCanonicalizer delegates to it — one definition of what a mode means instead of three.

#9 resolved itself: server/cors.js landed in #193, which is now this branch's base.

Verification

339 passing, 0 failing. eslint, markdownlint, cspell, prettier clean.

@flyingrobots
flyingrobots merged commit b265f84 into main Aug 16, 2026
7 checks passed
@flyingrobots
flyingrobots deleted the docs/contributor-accuracy branch August 16, 2026 03:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant