Claude Opus 5andbdeshi
7f9ac3c412
The verified-against sha could only ever be wrong. A commit cannot name itself,
so the line had to be written after the commit it described, which forced a
trailing `state:` commit every time — against conventions.md, which has always
said code, test, state.md row and ADR belong in one commit. Folding those
trailing commits away then left the sha naming a commit that no longer existed.
backup/pre-fold shows the pattern, and 8905686 is the commit that had to name
the survivor afterwards.
Git already knows when each file last changed. The gate now compares the last
commit touching docs/state.md against the last touching a .go file, and the
line is gone. Same intent, nothing to maintain, and no rewrite can invalidate it.
Also records two commit rules the human stated this session: state.md never gets
a commit of its own, and a minor change asked for just after a commit is amended
into it while it is unpushed rather than accumulating as noise.
133 lines
8.7 KiB
Markdown
133 lines
8.7 KiB
Markdown
# Conventions
|
||
|
||
The style floor. Do not ask about anything here — read it and comply. Disagreement is legitimate but
|
||
goes through an ADR, not a diff.
|
||
|
||
## Language and dependencies
|
||
- Go, current stable release. Stdlib first, every time.
|
||
- `net/http`, `html/template`, `log/slog`, `os`, `io/fs`, `embed`. Routing is `http.ServeMux` with Go
|
||
1.22+ patterns — `GET /{section}/{slug}/` and `{path...}` for multi-segment slugs — so no router
|
||
library is needed. No web framework, ORM, or config library — flags plus environment variables, parsed in one place. The site root
|
||
(`-site`, `KHOSRA_SITE`) is the only required setting; default templates are `embed`ded so a bare
|
||
site root renders (ADR-0011).
|
||
- New dependency = ADR + human approval + `scripts/allowed-deps.txt`. `verify.sh` enforces it.
|
||
- Prefer 40 lines of obvious code over a dependency doing it in one call — unless the 40 lines would be
|
||
subtly wrong in cases the author cannot predict, which is why YAML is a dependency (ADR-0020).
|
||
|
||
## Package layout
|
||
```
|
||
cmd/khosra/ main, flag parsing, explicit wiring — the only place things are assembled
|
||
internal/content/ bundles, frontmatter, slugs, queries — knows the disk, not HTTP
|
||
internal/render/ markdown, transforms, templates — knows content, not HTTP
|
||
internal/web/ handlers, routing, headers, caching — knows both, exposes neither
|
||
internal/ext/ extensions, one package each — earned at its counter (see extensions.md)
|
||
```
|
||
Dependencies point inward. `internal/content` imports nothing from the others; `cmd` imports everything
|
||
and is imported by nothing. A feature under `internal/ext/` may import `internal/content` and
|
||
`internal/render`, and must not import `internal/web`, `cmd/`, or **another feature** — sibling imports
|
||
are what make an agent's read set compound (ADR-0027). Every `internal/ext/*` package carries a `doc.go`;
|
||
`verify.sh` fails without one. Routes reach `web` by assembly in `cmd/khosra/wire.go`, so `web` never
|
||
learns features exist. No `utils`, `helpers`, `common`, `shared`, `manager`, `base`,
|
||
`impl`, `core` — a package name not describing a domain is a smell. Flat until a package exceeds
|
||
`FILE_LOC_WARN`; do not pre-partition, and a single-file package therefore warns at the same point it
|
||
wants splitting.
|
||
|
||
## Naming and shape
|
||
- Functions under `FUNC_LOC_WARN`, ideally under 20. Nesting depth under 4. File length is not a
|
||
target: one coherent file beats two split to satisfy a counter. Values: `scripts/budgets.env`.
|
||
- No `init()`. No package-level mutable state. No singletons. Wire explicitly in `cmd`.
|
||
- Accept interfaces only where a second implementation exists; return concrete types.
|
||
- `ctx context.Context` first when a call can block or be cancelled — not decoratively.
|
||
- Never call `time.Now()` outside a `clock.go`. A render that reads the clock is only true for a while,
|
||
and an injected accessor is what makes that expressible later; `verify.sh` fails on any other caller,
|
||
because a forgotten expiry serves staleness silently.
|
||
- Comments explain *why*, never *what*. Delete a comment narrating the next line. One stating a
|
||
non-obvious invariant is worth ten describing control flow.
|
||
|
||
## Documentation
|
||
|
||
Written for a maintainer working alone, years from now, with no agent to explain anything. `verify.sh`
|
||
fails on a missing package comment and on a citation naming an ADR that does not exist; a missing doc
|
||
comment on an exported identifier is a warning, because no gate can tell `// Load loads.` from a useful
|
||
sentence, and a gate whose cheapest satisfaction is noise buys noise.
|
||
|
||
- **Every package has a package comment.** What it owns, what it does not, and which packages may import
|
||
it. For `internal/ext/*` that is the four-line `doc.go` shape in `extensions.md`.
|
||
- **Every exported identifier has a doc comment** (warned, not gated). The exported surface of a
|
||
2000-line core is small, and
|
||
`go doc ./...` is the only navigation tool that still works when nothing else does. A comment that
|
||
restates the name (`// Load loads.`) is worse than none: say what it returns on absence, what it costs,
|
||
what it assumes.
|
||
- **Cite the ADR where the decision lives in the code.** `// Path shape: ADR-0008.` at the point that
|
||
builds a URL, `// Visibility inherits: ADR-0024.` at the guard. Twenty-seven decisions are unreachable
|
||
from code otherwise, and the next maintainer changes something whose reasoning they never saw.
|
||
`verify.sh` fails on a citation naming an ADR that does not exist.
|
||
- **Comment the trap, not the mechanism.** NFC normalisation, the settle window, parse order for template
|
||
overrides, the `old ∪ new` membership rule — each is a line of code that looks arbitrary and is not.
|
||
Those are the comments worth writing.
|
||
|
||
## Errors
|
||
- Wrap with `%w` at package boundaries, with operation and path: `parse %s: %w`.
|
||
- Never log and return the same error. Handle it, or return it.
|
||
- Request-time render failure degrades: log, serve what exists, never 500 on a missing field.
|
||
Startup failure is fatal and loud. Content authoring errors name the file and line.
|
||
|
||
## Tests
|
||
- **Behaviour ships with a test.** A change to `cmd/` or `internal/` carries a `_test.go` change in
|
||
the same commit; `verify.sh` fails otherwise. Test the observable contract, not coverage for its own
|
||
sake — one test proving the new behaviour is enough, and a refactor with no behaviour change needs
|
||
only the existing tests to still pass (touch them or say why not).
|
||
- Table-driven. Golden files in `testdata/`, regenerated behind a `-update` flag.
|
||
- Test the observable contract: URL in → bytes out; file on disk → page struct. Do not test private
|
||
helpers, and do not add a seam solely to make something testable.
|
||
- Needing a mock means the design is probably wrong — use a `testdata` directory with
|
||
`os.DirFS`/`fstest.MapFS`.
|
||
- One end-to-end test per route beats ten unit tests of render internals.
|
||
|
||
## Performance
|
||
Correct and small first; fast where measured. The render path is the only hot path.
|
||
- Write to `io.Writer`, never build pages by string concatenation.
|
||
- Parse templates once per rebuild, never per request and never inside a render method — the parsed set is
|
||
swapped whole so every page serves one snapshot, with no exception for `-dev` (ADR-0055, ADR-0056).
|
||
- No `sync.Pool`, caching layer, or goroutines in the render path until a benchmark justifies it and
|
||
the number goes in the commit message.
|
||
- No reflection in the hot path. `Extra` lookups are map reads, not reflection.
|
||
- Benchmarks live next to what they measure, added only when a decision depends on them.
|
||
|
||
## Frontend output
|
||
Semantic HTML working with zero JavaScript. Progressive enhancement only. No build step for CSS.
|
||
Page-specific styles and scripts come from the bundle (`styles`/`scripts` frontmatter). Respect
|
||
`prefers-reduced-motion`. Every image gets width, height, and alt. Payload discipline is a feature of
|
||
this project, not an optimisation.
|
||
|
||
## Git
|
||
One revertible unit per commit: what would be undone together belongs together, what would be undone
|
||
alone gets its own commit. Usually that is one feature — code, test, `state.md` row and ADR in one
|
||
commit, because reverting the code without the doc leaves a lie. A correction to something that was
|
||
already wrong beforehand is a separate unit even when it lands in the same sitting.
|
||
|
||
Imperative subject under 72 characters; body says *why*, and carries the numbers the change earned.
|
||
Committing happens by itself and needs no request (`CLAUDE.md` §4); pushing never does.
|
||
|
||
**Never a `state:` commit.** `state.md` is part of the change, not a follow-up to it — nothing about a
|
||
change is knowable only after committing it, now that currency is compared rather than declared
|
||
(ADR-0057). A doc commit of its own is for a correction to something that was already wrong.
|
||
|
||
**Amend rather than accumulate.** A minor change the human asks for just after a commit — a comment
|
||
removed, a wording fixed, a line they decided against — is amended into it while it is still unpushed.
|
||
History records units of work, not the order in which someone noticed things.
|
||
|
||
**Authorship names who wrote the bytes, not who asked for them.** Three cases, no fourth:
|
||
|
||
| Who wrote the change | `Author` | Trailer |
|
||
|---|---|---|
|
||
| the agent alone | `Claude Opus 5 <noreply@anthropic.com>` | none — the author field already says it |
|
||
| both | the human | `Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>` |
|
||
| the human alone | the human | none |
|
||
|
||
Directing a change is not writing it: a one-sentence request that the agent implements in full is
|
||
agent-authored. The committer stays the human in every case, and so does the signature — the key
|
||
attests to taking responsibility for the commit, which is a different claim from having written it.
|
||
`git log --author=Claude` is then an honest answer to "how much of this did the agent write", which is
|
||
the whole point of recording it.
|