feat(lint): a comment budget rule, and what calibrating it against our code showed

[dawsontoth]

Adds comment-budget, a custom oxlint rule that warns when a file carries more than 12 comment "sites" or a single block more than 4, to push toward code that explains itself rather than prose that drifts from it. It is a warn on an experiment footing — CI and the pre-commit hook stay green — and nothing off the shelf does this (the two npm packages that come close are abandoned 2022 stubs, one of which enforces a minimum comment density).

Scoped to this repo only; the rule is written to the ESLint v9 API and would need no changes to become a package later, but nothing here depends on that.

For the human reviewer

1. Is this worth having at all? kriszyp's review makes the strongest counter-argument and I largely agree with it: for newly written code the skills updates apply comment discipline at write time, which beats flagging it afterwards. What is left for a lint rule is pre-existing code, human edits made outside the harness, and a trackable number. Sampling my own flagged set (6 of 52 blocks, chosen before reading them) put the precision at 2 clear hits — it flags load-bearing prose about as often as narration. Reversibility is the argument for merging anyway: 140 lines and one config entry to delete, and the calibration data is the durable artifact either way. A "no" costs nothing already spent. Approved by kriszyp on the experiment framing; ruling again here is fair game.
2. Sites, not lines or comments. A run of adjacent own-line comments is one site however long. Budgeting per line would price a considered six-line explanation above six scattered // increment is, which inverts the incentive. The cost is that verbosity within one comment is unpriced. Reversible in ~10 lines of toSites.
3. 12 per file / 4 per block, from measurement not taste. Across src: median 3 sites per file, p90 9, p95 12. At 12/4 that is 78 warnings; 10/2 would be 175. I picked the looser end because the baseline was previously spotless and I did not want to spend that signal. Tightening is a one-line config change.
4. warn, not error. oxlint exits 0 on warnings, so this cannot fail CI or the hook. The cost is 78 permanent warnings eroding a previously clean baseline. Ratcheting later is the intended path, not shipping strict now.
5. The block signal is really a scope-size signal. The densest scopes here (useResizableDialog.ts, DatabaseTableView.tsx) are the best-commented code we have — Radix ref-loop hazards, the describe_all/describe_table race, a #1199 reference. They trip the budget because the functions are large. max-lines-per-function measures that more directly with no custom plugin, and choosing this over that is a real call I could be wrong about. AGENTS.md states plainly that answering a warning by deleting prose leaves us worse off.
6. Test files share the one budget. Tests hold the extreme tail (220 of 596 commented files; max 40). An oxlint overrides block giving them their own budget is a few lines; I left it out because a second knob is easier to add on evidence than to remove. Watch for a few weeks.

Verification

Route: new unit tests plus a repo-wide run of the rule against real code — a lint rule is fully observable at the linter, so there is no integration surface beyond that.

• 20 rule tests, driven through the real oxlint binary rather than by calling create() directly, so they exercise oxlint's own AST and SourceCode rather than my assumptions about them. Covers budget arithmetic, paragraph grouping, the blank-line break, trailing comments, CR line endings, directive and JSDoc exemption including TS triple-slash, banner comments, annotated array/object literals with both the own-line and nested-block carve-outs, prose that merely opens with a tool name, sibling literals charged separately, innermost-block attribution, switch-case scoping, one-report-per-scope, and the inline disable.
• Every fix was checked against its own counterfactual rather than asserted — reverting the fix must turn its test red. CR: 2 sites instead of 1. Triple-slash: 1 instead of 0. Both exemption holes: [] instead of a 4-comment block warning. A regression test that passes either way proves nothing.
• The two exemption holes were reproduced as live defects before being fixed: a five-comment object method body and five prose comments opening with tool names each produced zero warnings where five were due.
• Repo-wide: pnpm lint reports 78 warnings (29 file, 49 block) and exits 0 — unchanged by the exemption fixes, so neither hole was being exploited here; both were latent. pnpm test 292 files / 2,275 tests green, tsc -b clean, dprint check clean. Studio has no test:unit:main / test:integration:all scripts; those are the repo's full-gate equivalents.
• Two boundary conditions checked rather than assumed: jsPlugins exists in the oxlint config schema back to our declared ^1.32.0 floor, and a broken plugin path fails the whole lint run with exit 1 rather than silently dropping the rule.

Review coverage

Authored by Opus 5. Cross-model review: codex (gpt-5.6-sol) ✓, gemini via agy (default model) ✓. Failed or skipped, named rather than omitted: cursor-composer ✗ — cursor-review structurally refuses any diff that changes agent instructions, and this one edits AGENTS.md; cursor-grok ✗ pruned by policy; Harper domain adjudication ✗ (exit 1 twice, then pruned on the narrow deltas). So outside coverage here is two lenses, not four, and no adjudication ran. Receipt @ 6e068c79.

Five rounds. Rounds 1–4 found five real defects — each reproduced as a live failure before being fixed, each with a counterfactual proving its regression test goes red without the fix. Round 5 returned no findings.

Worth stating plainly: this review should have run before the first push. It did not. The four findings that reached this PR from gemini and kriszyp were all the same class the pre-push leg catches in minutes, and running it late turned up five more that no PR reviewer had seen.

_{Human-Review-Need: 4 @ 6e068c7}

[gemini-code-assist]

Code Review

This pull request introduces a custom oxlint/ESLint plugin, comment-budget, which enforces a configurable limit on the number of comment sites per file and per block. It includes documentation in AGENTS.md, configuration in .oxlintrc.json, and a comprehensive test suite. The review feedback highlights three key improvement opportunities: making the line-boundary check in isOwnLine robust against carriage returns (\r), optimizing the O(N^2) array spreading inside the block-grouping loop to O(N), and appending .cmd to the oxlint binary path on Windows to ensure cross-platform test execution.

[github-actions]

Coverage Report

Status	Category	Percentage	Covered / Total
🔵	Lines	55.45%	6701 / 12083
🔵	Statements	56.05%	7212 / 12865
🔵	Functions	47.66%	1661 / 3485
🔵	Branches	49.64%	4715 / 9497

File Coverage

No changed files found.

Generated in workflow #1750 for commit 6e068c7 by the Vitest Coverage Report Action

[kriszyp]

Kinda wanted to see some examples, since I am skeptical. Although I guess if the point is to try this on studio as the "experiment", that's fine, go for it.
(And I as mentioned, I feel like the problem has been completely solved with the latest skills updates)
_{🤖 Reviewed with Codex}

[dawsontoth]

Fair ask, and the honest answer is less flattering to the rule than I'd like. I read 6 of the 52
flagged blocks rather than picking the ones that look good. 2 of the 6 are cases where the rule is
clearly right.

Where it earns its keep — src/lib/string/collapseKebabsToMaxLength.ts, 8 sites in one block:

// If the string is already within the max length, return it as is
if (str.length <= maxLength) { return str; }

// Split the string by hyphens to get individual segments
const segments = str.split('-');

// If there's only one segment, just truncate it
if (segments.length === 1) { return segments[0].slice(0, maxLength); }

Every comment restates the line under it in English. Same shape in to-kebab-case.ts, which has
// Convert to lowercase sitting on top of .toLowerCase(). This is the drift risk you'd want
caught: nothing verifies those sentences, and they're pure cost.

Where it is wrong — src/integrations/datadog/shouldKeepEvent.ts:109, 10 sites, all like this:

// Errors thrown entirely inside an embedded third-party script are that vendor's bugs, not
// Studio's: we can't reproduce them, fix them, or act on them... Reo.dev alone contributed two
// distinct "issues" within a day of first appearing (2026-07-28) ... Attribute on the stack
// rather than the message, so a genuine Studio error that happens to share a message is kept.

No identifier carries that. Same for components/ui/table.tsx (why relative and not
overflow-hidden, why select-none).

And a failure mode I hadn't anticipated — src/lib/string/entityId.test.ts:32, 9 sites:

'org',        // no body
'org-',       // empty body
'Org-1',      // uppercase prefix is a title, not an id
'org 1',      // space, not a hyphen

An annotated test-case table. These are good comments, and my own "trailing comments never merge"
rule — added to fix a different bug — is exactly what makes each one cost a full site. So a
well-documented fixture table is the single most expensive thing you can write under this rule.

That suggests a concrete refinement before anyone takes the numbers seriously: exempt (or merge)
trailing comments on elements of an array/object literal, since annotating data is not the same act
as narrating control flow. Happy to add it — it would clear entityId.test.ts and probably several
of the 52.

On skills having solved this

I think you're mostly right, and I'd rather say so than argue the rule up. For newly written code the
skill applies at write time, which is strictly better than flagging it afterwards. What a lint rule
adds is narrower than I implied when I opened this: it covers code that predates the skills and
human edits made outside the harness, and it produces a number that can be tracked over time.

But the sample above says its precision as a comment-quality signal is poor — it flags
load-bearing prose at least as often as narration. Its defensible value is the scope-size signal, and
max-lines-per-function measures that more directly with no custom plugin.

So the experiment framing is the right one. A fair test over the next few weeks is simply: does
anyone actually change code because of a warning, and is what they change better afterwards? If the
answer is no, the rule is 140 lines and one config entry to delete, and the calibration data in this
PR is the useful artifact either way.

🤖 Generated with Claude Code

[kriszyp]

Be the guinea pig!

[dawsontoth]

Implemented the data-literal refinement I floated above — f7764f08.

A trailing comment inside an array, object, tuple, type literal, or enum body now charges that
literal once, however many rows carry a note. Own-line prose inside a literal is deliberately
not covered and still follows the paragraph rules, so this can't be used to park commentary inside
an array.

82 warnings → 78, and the three that cleared were all annotated tables, which is exactly the
class I wanted gone:

File	Before	After
lib/string/regex/hostNameRegex.test.ts	block:15, file:15	clean
lib/string/entityId.test.ts	block:9	clean
analytics/lib/csvExport.test.ts	block:5	clean

hostNameRegex.test.ts was the second-densest block in the repo, and it was 15 rows of
'-bad.com', // starts with hyphen in label. Under the old rule that was the single most expensive
thing in the codebase; now it costs one site. Nothing outside test fixtures changed, which is the
right blast radius.

Four new tests (17 total): array literals, object literals, sibling literals charged separately, and
the own-line carve-out that keeps the exemption honest. Also confirmed TSEnumBody genuinely matches
in oxlint's AST rather than being a dead selector I'd added on faith.

Re-running the earlier sample with this in place, the 2-of-6 hit rate becomes 2-of-5 — the
entityId.test.ts false positive is simply gone rather than reclassified. The two genuine misses
(shouldKeepEvent.ts, components/ui/table.tsx) are still misses, and I don't think any amount of
heuristic fixes those: they are long functions full of irreplaceable prose, which remains the honest
limit of this rule.

🤖 Addressed by Claude Code