One idea at a time: definition, necessity, priority, layer, recommendation. Every row is now shipped, scheduled, parked with a trigger, or dropped with a reason — so both tracking files are gone rather than left as a parallel backlog. Roughly a third already shipped. A quarter needed only a theme fragment or CSS, including several the list assumed were features: arbitrary `theme.*` frontmatter attributes already reach templates through Extra (verified), native lazy loading already ships and is deliberately absent on lead figures, and in-page timelines are a theme-defined container. roadmap.md gains an "Order of work" — nine bodies of work in sequence, because the sections this review added had accumulated without one. Item 0 is a decision, not work: core sits at ~2965 of 3000 and logging alone wants the remainder, so discover.go moves out *and* the ceiling rises with an ADR. feed.go and web/extras.go cannot move; they are resolver cases, and three parked items now wait on that same seam decision. Two ADRs came out of it. ADR-0083: the dependency rule described a practice it forbade. "Stdlib first, always" and "usually 40 lines of stdlib" would, read literally, have argued against all five modules here — every one is a format or algorithm somebody else specified, and their allowlist comments say so. Surfaced when this agent proposed hand-rolling Reed–Solomon for QR encoding. The test is now whether you can verify it, not whether you can write it, with compute-versus-hold as the counterweight. conventions.md and allowed-deps.txt contradicted it and were realigned. ADR-0084: the Effect primitive said every Effect is "idempotent and re-runnable from scratch" — true of derivatives, false of anything outbound, since a sent message cannot be recalled. Split into artifact-producing and outbound, the latter idempotent only against a delivery ledger, at-least-once with a dedupe key, and ledgers in a -state directory distinct from the disposable cache. Asked for as forward-looking design so webmentions need no redesign; recorded as a shape rather than built, the way extensions.md records the Extension struct. Also fixed a STATUS claiming Effects were "not buildable yet" when the derivative pass has inhabited them since ADR-0042. Findings that were defects rather than ideas: the Atom feed emits no <author>, which RFC 4287 requires. HTML comments in content are published verbatim — verified — so anything commented out is already public, and a check warning is scheduled rather than the engine deleting authored bytes. `Page` has no Date, so a theme cannot mark up dt-published or show an article's own date. A frontmatter naming rule after this agent proposed `archive` alongside the accepted `archived`: no near-homograph keys, and name the benefit rather than the vendor. reference/microformats-and-indieweb.md is new and is the reference asked for — microformats2 properties and IndieWeb rel values mapped to where a theme puts them, with spec URLs, marked scheduled. rel="me" is the highest-value lowest-cost item in that space and needs no endpoint at all. Four things this agent got wrong and the human caught: asserting "the engine never fetches" from a sentence scoped to the content repo, twice; inventing a data-sovereignty argument for an item he had filed as "a cool geeky thing", when the raw Markdown in his git already is the sovereignty; the archive naming; and claiming microformats could be fully implemented today. 15 files, +556/-385. No code changed. Nothing was scheduled that this agent could not name a consumer for. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
10 KiB
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, with the balance
CLAUDE.mdrule 2 states (ADR-0083): what you can verify by reading it, not everything you could technically write. net/http,html/template,log/slog,os,io/fs,embed. Routing ishttp.ServeMuxwith 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 areembedded so a bare site root renders (ADR-0011).- New dependency = ADR + human approval +
scripts/allowed-deps.txt.verify.shenforces it. - Prefer 40 lines of obvious code over a dependency doing it in one call — unless those lines would be subtly wrong in cases the author cannot predict (why YAML is a dependency, ADR-0020), or the task is one somebody else specified, where the work is verification rather than authorship (ADR-0083).
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 incmd. - Accept interfaces only where a second implementation exists; return concrete types.
ctx context.Contextfirst when a call can block or be cancelled — not decoratively.- Never call
time.Now()outside aclock.go. A render that reads the clock is only true for a while, and an injected accessor is what makes that expressible later;verify.shfails 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.
- No two frontmatter keys may be near-homographs.
archiveandarchiveddiffer by one letter and would have meant "submit this page's outbound links" and "this content is stale" — proposed in the same review and caught only by the human. A key is a word an author types from memory months later, so distinctness matters more than brevity. Name the benefit, never the vendor:preserve_links, notwayback. - Shell scripts end in
.sh, with exactly one exception:scripts/hooks/pre-commit. git locates a hook by exact filename, so a.shsuffix makes it invisible and the gate stops running on every commit while still appearing to exist — verified by renaming it in a throwaway repository and watching a commit succeed with the hook silent. The name belongs to git, not to this convention. Consequence worth remembering: a repo-wide sweep filtered by--include='*.sh'walks past that file, which is how a stale path survived in it (ADR-0082), soverify.shnow checks the paths insidescripts/whether or not they are quoted.
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-linedoc.goshape inextensions.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.shfails 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 ∪ newmembership rule — each is a line of code that looks arbitrary and is not. Those are the comments worth writing.
Errors
- Log levels mean these things, and
Erroris not "look at this".Error: the engine could not do what was asked — a request or build step failed, startup is aborting.Warn: it worked around something wrong in content or config and still served, which is all of ADR-0029's category.Info: lifecycle and one line per request.Debug: off by default, per-request detail while diagnosing. A log line addresses an operator, not its author; if it cannot be acted on, it isDebugor it is nothing. - Wrap with
%wat 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/orinternal/carries a_test.gochange in the same commit;verify.shfails 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-updateflag. - 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
testdatadirectory withos.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.
Extralookups 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.
| 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.