Docs: split the README into an introduction and a reference manual by flavorjones · Pull Request #94 · basecamp/hotcell

flavorjones · 2026-10-02T21:20:13Z

Motivation

The README ran to 681 lines and covered everything from the pitch to the log schema, and discussion #86 called it verbose. Each file in docs/ held rationale, how-to steps and settings tables together, and some facts appeared in several files: the OpenMP bound was in the README, docs/DEPLOYMENT.md and docs/IMAGEMAGICK.md, and the timeout rule was in both docs/DEPLOYMENT.md and docs/TUNING.md.

Nothing tied a page to the code it described. docs/DESIGN.md records that an earlier API description went stale and its errors spread back into code comments.

Details

Layout. The README is a human introduction: what HotCell is, an Active Storage quick start, a short custom-operation example, and one line per recommended alert, each linking to a reference page. Everything else is a reference manual under docs/, one topic per page, written in Google developer documentation style. docs/DESIGN.md is split into docs/design/, with the invariant and experiment numbers unchanged.

Frontmatter. Each page under docs/ starts with Open Knowledge Format (OKF) frontmatter: type, title, a one-line description, and sources, the files or directories that the page describes. docs/index.md lists every page with its description, so an agent can choose a page without opening the others.

Tooling. Three rake tasks in rakelib/docs.rake:

rake docs:stale lists each page whose sources changed since BASE (default origin/master) while the page did not. It never fails. A new Docs CI job prints the list as warnings on a pull request.
rake docs:index regenerates the page list between the  markers in each index.md.
rake docs:check fails on a page without type, title or description, on a source that does not exist, and on an out-of-date index. The Docs job runs it on every push, and so does rake.

New docs_test.rb files in hotcell-core, hotcell-server and activestorage-hotcell-server fail when the codes and kill-causes table, the log events table, the cell defaults tables or the shipped operation limits table disagree with the code.

During development. After changing code, run rake docs:stale, read each page it names, and fix what the change made wrong. Edit sources when a page starts or stops describing a file, and run rake docs:index after adding a page or changing a title or description. AGENTS.md says the same under "Keep the docs current", along with the writing rules for each kind of page.

Corrections. The README's Active Storage initializer did not call HotCell.register "active_storage", which every shipped client needs. docs/DEPLOYMENT.md said that an unset HotCell.root runs callers in process; perform_in_hotcell raises HotCell::CellNotConfigured. Both are fixed.

Additional information

The commits are meant to be reviewed one at a time:

Moves every section, unchanged, into its new file, and repoints the code comments that cited the old paths. A script assigned every source line to exactly one destination; only the tables of contents were dropped.
Rewrites the README and the reference pages, merges duplicates, and adds reference that no page had: response codes, HotCell::Operation, Input and Output, and the client's boot checks. The design pages keep their prose; only headings and links changed.
Adds the frontmatter, the rake tasks, the CI job, the tests and AGENTS.md.
Restores details that the rewrite dropped, found by an adversarial review and by comparing every old sentence with the new pages.

The README is 331 lines; most of what remains is the Kamal configuration, which a reader copies.

The README and each file in `docs/` held both guide prose and reference material, and some facts appeared in more than one file. Move each section of the README, `docs/DESIGN.md`, `docs/DEPLOYMENT.md`, `docs/TUNING.md` and `docs/LOGS.md`, unchanged, into one file per topic under `docs/`, and repoint the code comments that cite those files. Links between the moved sections stay broken until the next commit.

The README ran to 681 lines, and each reference page held rationale, how-to steps and settings tables together. The README's Active Storage initializer also omitted `HotCell.register`, and the deployment guide said that callers run in process when `HotCell.root` is unset, but `perform_in_hotcell` raises `HotCell::CellNotConfigured`. Cut the README to an introduction and a quick start that links to the reference pages. Rewrite each reference page in Google developer documentation style, keep each fact in one place, correct both statements, and add reference for what no page covered: the response codes, `HotCell::Operation`, `Input` and `Output`, and the client's boot checks. Leave the design pages' prose as written.

Nothing connected a reference page to the code it describes, so a change to the code left the page wrong without notice. Give each page OKF frontmatter with a description and the `sources` it describes. Add `rake docs:index` to generate the page lists, `rake docs:check` to fail on missing frontmatter, missing sources and stale indexes, and `rake docs:stale` to name the pages whose sources a branch changed. Run the check and the stale list in a new CI job, add tests that compare the codes, events, defaults and shipped limits tables with the code, and say in `AGENTS.md` how to use all of it.

The rewrite dropped the README's RPC framing, its extensibility list and a few development notes, dropped the performance side of `max_requests_per_worker`, and overstated when a `protocol` mismatch heals. Restore them, and say that `protocol` heals when the accessory reboots on a matching image.

Copilot

Copilot review overview

🟡 Changes recommended

Several operational rules and API examples contradict the implementation and could cause failures or unsafe scratch sizing.

Review effort: Balanced
Findings: 13

Open (13)

Clarify permanent verdicts can be fixed by deployment tuning · New
Qualify claim that supervisor never reads requests · New
Correct nonexistent scratch subdirectory layout · New
Account for multiple files in scratch sizing · New
Avoid marking fd_path output as staged via Output#path · New
Document worker.undispatchable request inspection exception · New
Do not promise retries for all full-filesystem failures · New
Avoid underprovisioning scratch for multi-file requests · New
Include all concurrent request files in scratch sizing · New
Document per-file rather than uncapped file_size limits · New
Qualify full-filesystem failure classification · New
Size scratch for aggregate per-request usage · New
Describe operation limits as conditional failure thresholds · New

What changed in this PR

Splits the monolithic documentation into a concise README and source-linked reference manual, with validation tooling and synchronization tests.

Changes:

Adds topic-focused reference and design pages.
Adds documentation indexing, staleness checks, CI, and table synchronization tests.
Updates code comments and contributor guidance to reference the new structure.

Pull request overview

[!TIP]
If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or run gh pr ready --undo.
Click "Ready for review" or run gh pr ready to reengage.

File	Description
`.github/workflows/ci.yml`	Adds documentation CI.
`AGENTS.md`	Documents the reference workflow.
`CHANGELOG.md`	Records the documentation restructure.
`CONTRIBUTING.md`	Updates documentation guidance.
`README.md`	Becomes the introduction and quick start.
`Rakefile`	Adds documentation validation to defaults.
`activestorage-hotcell-server/test/docs_test.rb`	Verifies documented operation limits.
`bin/conformance`	Updates documentation references.
`docs/DEPLOYMENT.md`	Removes the former deployment guide.
`docs/DESIGN.md`	Removes the monolithic design document.
`docs/IMAGEMAGICK.md`	Replaced by the new reference page.
`docs/LOGS.md`	Replaced by observability documentation.
`docs/TUNING.md`	Replaced by the rewritten tuning page.
`docs/active-storage.md`	Documents Active Storage operations.
`docs/cell-settings.md`	Documents cell configuration.
`docs/client-api.md`	Documents the client API.
`docs/codes.md`	Documents response classifications.
`docs/concepts.md`	Defines core terminology.
`docs/conformance.md`	Documents image conformance checks.
`docs/container.md`	Documents container configuration.
`docs/design/descriptors.md`	Records descriptor design rationale.
`docs/design/experiments.md`	Preserves numbered experiments.
`docs/design/index.md`	Indexes design documentation.
`docs/design/invariants.md`	Preserves numbered invariants.
`docs/design/overhead.md`	Records overhead findings.
`docs/design/threat-model.md`	Separates the threat model.
`docs/design/worker-isolation.md`	Documents worker isolation.
`docs/imagemagick.md`	Rewrites ImageMagick guidance.
`docs/index.md`	Indexes the reference manual.
`docs/observability.md`	Consolidates logs and metrics guidance.
`docs/operation-api.md`	Documents operation and descriptor APIs.
`docs/request-lifecycle.md`	Documents request processing.
`docs/scratch.md`	Documents scratch layouts and sizing.
`docs/tuning.md`	Rewrites tuning guidance.
`examples/gate`	Updates documentation references.
`hotcell-client/lib/hot_cell/client.rb`	Updates client API reference.
`hotcell-client/test/describe_survival_test.rb`	Updates design reference.
`hotcell-core/lib/hot_cell/codes.rb`	Updates isolation reference.
`hotcell-core/test/docs_test.rb`	Verifies documented response codes.
`hotcell-server/lib/hot_cell/log.rb`	Updates observability reference.
`hotcell-server/lib/hot_cell/operation.rb`	Updates operation reference.
`hotcell-server/lib/hot_cell/slot.rb`	Updates isolation reference.
`hotcell-server/lib/hot_cell/supervisor.rb`	Updates design and logging references.
`hotcell-server/test/docs_test.rb`	Verifies events and defaults.
`hotcell-server/test/log_test.rb`	Updates log schema reference.
`rakelib/docs.rake`	Adds index, check, and stale tasks.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

+- **Permanent:** the same request fails the same way until the input or the code changes. A change in load
+  or deployment doesn't fix it. A caller can record a permanent failure against the input, for example
+  against an Active Storage blob, and serve it from a cache.


+The supervisor never reads a request and never evaluates image data. It passes the accepted connection
+itself to a worker over `SCM_RIGHTS` and never calls `recvmsg`, so the caller's descriptors are still
+queued on the connection when the worker reads them.


+## Slot
+
+A slot is the numbered workspace that a worker borrows. A slot holds one directory for each request. That
+directory is the request's `$HOME`, and the request's staged files are in its `scratch` subdirectory.


+| `cpus` | `2` | The share of the host that this cell can use. Start the cell's `concurrency` at twice this number, and match the image's `OMP_NUM_THREADS` to it. See [Bound the OpenMP thread pools](#bound-the-openmp-thread-pools). |
+| `memory` | `2g` | The cgroup limit, which counts every worker and the tmpfs. Size it from `concurrency × peak RSS` plus the tmpfs. Keep it above the cell's `memory`. On a disk-backed scratch, there's no tmpfs term. See [Scratch](scratch.md). |
+| `memory-swap` | `2g` | Set it equal to `memory`. If you omit it, Docker allows twice `memory` in swap, and the memory limit no longer holds. |
+| `tmpfs` size | `size=512m` | Scratch for all concurrent workers together. It pairs with `file_size × concurrency`. Moving scratch onto disk separates it from `memory`. See [Scratch](scratch.md). |


+
+    # The result: one JSON object, which the caller receives as perform_in_hotcell's return value (in
+    # addition to the destination file descriptor)
+    { format: format, bytes: File.size(destination.path) }


+The `file_size × concurrency` constraint in [Tuning](tuning.md#constraints-between-the-numbers) applies
+to every layout:


+If you also raise or remove `file_size` so that large conversions succeed, a cap is the only bound left on
+what a runaway write consumes. Without a cap, the bound is deadline × disk throughput. An uncapped layout
+with an uncapped `file_size` is the one combination with no bound at all, and a capped filesystem is what
+makes a generous `file_size` safe to run.


+
+- `HOTCELL_WORKSPACE` keeps its default. It's under `Dir.tmpdir`, and the server treats scratch as a plain
+  directory without checking the filesystem type.
+- A full scratch still reaches the caller as `failed`, which is transient, as a full tmpfs does.


+- `file_size × concurrency` must be no more than the scratch. Above it, concurrent workers fill the
+  scratch, and requests fail with `ENOSPC` instead of with a limit verdict. On the default accessory, the
+  scratch is the tmpfs, and its `size=` is the number to fit. See [Scratch](scratch.md#what-changes-in-the-numbers).


+Size the cell to its most demanding operation. The cell clamps each operation's own `limits` to its own,
+so the cell's numbers only ever take away. Read the `limits` that each operation you carry declares, and
+set the cell above the highest of them. The shipped video previewer asks for `deadline: 120` and
+`file_size: 128MB`. A cell configured with 30 seconds and 48MB kills every video preview and nothing else,
+which is a hard failure to place. See [Active Storage operations](active-storage.md#limits).


flavorjones added 4 commits October 2, 2026 16:47

Copilot AI balanced review requested due to automatic review settings October 2, 2026 21:20

Copilot started reviewing on behalf of flavorjones October 2, 2026 21:20 View session

Copilot AI reviewed Oct 2, 2026

View reviewed changes

Navigation Menu

Docs: split the README into an introduction and a reference manual - #94

Docs: split the README into an introduction and a reference manual#94
flavorjones wants to merge 4 commits into
masterfrom
card-577-prep-docs-v1

flavorjones commented Oct 2, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		The `file_size × concurrency` constraint in [Tuning](tuning.md#constraints-between-the-numbers) applies
		to every layout:

Conversation

flavorjones commented Oct 2, 2026

Motivation

Details

Additional information

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Pull request overview

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants