Stirling-PDF/devGuide/CODE_COMMENTS.md

Code comments

A comment must carry information the code cannot. If a reader could derive it from the code in front of them, delete it: a redundant comment still has to be maintained, will eventually contradict the code, and dilutes the comments that matter.

The operative rules are in AGENTS.md, kept short so they stay in an agent's context. This document is the reasoning and the worked examples behind them, plus how to run the linter.

Comment the current state

Describe the code as it is. Not what it used to be, not what changed, not why it changed. A comment that narrates history is stale the moment the next change lands, and git already holds that record.

When you know the history and it explains the shape of the code, the useful half is the reason, not the sequence. State the reason:

// Don't:
// This used to reimplement the modal internals, which is how the procurement
// dialogs drifted from the billing ones.

// Do:
// Thin wrapper over the shared Modal: duplicating its portal and focus trap is
// how dialogs drift apart.

Future state is the exception, and it belongs in a TODO with an issue.

The four jobs

Contract. What a caller must know that the signature cannot say: preconditions, invariants, units, ownership and lifetime, thread-safety, error semantics, side effects.

The bound is the surface, not the volume: document the contract of everything a caller outside the file can reach, and nothing else. Inside that surface say whatever a caller needs; outside it a comment earns its place on the same terms as any other.

/**
 * Authority on which filesystem locations a policy may read or write. Fail-closed
 * in order: denied entirely under the saas profile; Stirling's own config dir is
 * always rejected; the path must resolve within policies.allowedFolderRoots.
 *
 * <p>Compared after normalisation so {@code ..} cannot escape a root. Symlink
 * escape is not defended: an operator who roots an allowlist on a symlink to a
 * sensitive location is trusted.
 */

Why. The constraint the code satisfies, the bug it avoids, the alternative rejected and the reason.

// whenComplete runs on the worker thread after the run finishes, so the
// terminal event never races the step events.
handle.completion()

A reference is supplementary, never load-bearing: the comment must survive deleting it. // See #1234 is a dead end.

// flatten() reads the annotation list that save() clears, so saving first loses
// every annotation (#6865).
document.flatten(annotations);

Prefer a spec (RFC 3161, ISO 4217) or a CVE where one applies. Both are immutable; a ticket can be closed, moved or made private.

Hazard. "Must stay in sync with X." "Order matters because Y." "Do not remove, it prevents Z."

Map. A short orientation at the head of a genuinely complex file: what it owns, and what it deliberately does not.

The test that decides it

A comment earns its place when it sits at a different level of detail than the line below it: lower, stating a precise fact the code implies but does not say, or higher, giving intent a reader would otherwise assemble from ten lines. Same-altitude is the definition of redundant.

• Delete it. Is any information lost? If not, it stays deleted.
• Could a name carry it instead? A better identifier, an extracted function or a named constant beats a comment. Prefer the code change.

What not to write

Don't	Instead
// Handle drag start above handleDragStart	Nothing. The name already says it.
// ─── Types ───, // Helpers, // ====	If a file needs internal signposting, split the file.
// Step 1: narrating a function body	Extract functions. If the steps need labels they need names.
// No longer needed, // Previously this used X	State why the code is as it is now, or nothing.
Commented-out code	Delete it. Git remembers.
@param blob - The blob to download	Omit the tag rather than pad it.
Docs on a self-explanatory member	Nothing, unless there is a real constraint to state.

Step numbering is fine where it labels a genuinely numbered thing, such as a wizard step or a step in a written test procedure. It is narration when it numbers the lines of one function.

Comments at the end of a line

A trailing comment usually does a different job from one above the code: it decodes the line it sits on. Those are worth keeping, and the linter leaves them alone.

byte[] pdfBytes = {0x25, 0x50, 0x44, 0x46};   // "%PDF"
long maxAttachmentSize = 50L * 1024 * 1024;   // 50 MB
double buffer = 0.10;                         // 10% headroom
default -> toBytes(value, 2);                 // MB

Each overlaps in words with the code and each adds the interpretation the code leaves implicit, which is the lower-altitude case the test above asks for. So CMT001 does not judge trailing comments; on this codebase it would have been wrong about roughly six in seven of them.

What still applies is anything that does not depend on the code below: a trailing // TODO fix this is as unowned as one on its own line, and a trailing // this used to run before the flush narrates history wherever it sits.

A comment block over about 12 lines, outside a file or type header, is usually a sign the code needs restructuring. If it is genuinely product documentation, it belongs in the docs repo.

TODOs

A TODO needs an issue, because an issue is the only part that will close it:

// TODO(#1234): re-enable the checkout gate once account syncing lands

An owner is not a substitute: a username goes stale when someone changes team and means nothing to an outside contributor. If the work is not worth an issue it is not worth a TODO, and the options are to do it now or leave the code alone. A question is not a TODO.

Per language

Java. Google Java Style, which this repo already formats to. Its §7.3.1 exception applies: omit Javadoc on a self-explanatory member where there is genuinely nothing to add, but do not cite it to skip something a reader needs. Summary fragments are noun or verb phrases, not sentences starting "This method returns".

TypeScript. JSDoc on the @app/* seams, exported hooks, and anything crossing a layer boundary. No @param/@returns that restates a typed signature. JSX comments follow the same rules as any other.

Python. Docstrings on modules, public functions and Pydantic models where the contract is not obvious from the type.

The linter

task comment-lint          # what the working tree adds over HEAD
task comment-lint:branch   # what the branch adds over origin/main (BASE=<ref> to change)
task pre-commit:comment-lint:ci    # the fixture corpus, then the diff

comment-lint is the pre-commit question, so it reports nothing once you have committed; on a CI pull request it compares against the target branch via GITHUB_BASE_REF. comment-lint:branch is the review question. The corpus checks the rules themselves rather than the code under review, so it runs on CI and before a rule change, not on every local commit.

task comment-lint also runs inside task pre-commit, and as a Claude Code Stop hook, so an agent is told before it finishes a turn and fixes the comment inside that turn. Stop rather than per file write: a run costs the same for one file as for twenty-five, and half of all writes in a turn go to a file already written in it.

Findings are scoped to comment text that is new, not to lines git calls new, so reindenting or moving code does not resurface comments you did not write.

The rules are the RULES object in scripts/lint/comment-rules.mjs; the exact condition for each is the predicate of the same name in that file, with the readings it deliberately excludes beside it.

Every rule blocks. A rule that only warns is a rule nobody acts on. So a finding you believe is wrong is a bug in the rule, not something to live with: narrow the rule, or mark the line and say why.

Every comment form the repo writes is covered: // and /* */, Javadoc and JSDoc, JSX comments, #, and Python docstrings. CMT007 reads all three parameter conventions in use here, Javadoc/JSDoc @param, Sphinx :param name: and Google name: description under Args:.

Two engines, one rule set. .ts/.tsx/.mjs go to an oxlint JS plugin, so comments come from the parser: a // inside a string is not a comment, and JSX {/* … */} is. .java/.py go to a line scanner. Neither reads the other's files, so they cannot disagree about one file. scripts/lint/fixtures/ is the corpus that keeps them meaning the same thing.

When a finding is wrong

Name the rule on the line above:

// comment-lint-allow: CMT002
// ─── kept deliberately, because <reason> ───

There is no form that disables every rule, and the directive has to earn its place. CMT008 reports one that names something which is not a rule, and one that silences nothing, so a typo does not read as a suppression and a stale suppression does not sit there blinding the line. The whole comment must be the directive; prose that mentions the syntax is just prose.

If you reach for this more than occasionally the rule is wrong: fix it in comment-rules.mjs and update the fixture corpus in the same commit, so the diff shows what moved.

The existing backlog

task pre-commit:comment-lint:all reports the whole tree and never fails. There is a standing backlog being cleared by directory; diff scoping is what keeps it off whoever touches a file first.

To turn the editor hook off, put { "env": { "COMMENT_LINT_HOOK": "0" } } in .claude/settings.local.json. The commit-time gate still applies, so you lose the early warning rather than the check.