refactor: remove the inline comments the codebase was told never to carry - #256
Merged
Conversation
…arry AGENTS.md has forbidden inline code comments since it was written. 1,743 comment lines had accumulated across 189 source files anyway, 1,281 of them written by agents, because nothing loaded the rule and nothing checked it. #254 fixed the second half. This is the backlog it left behind. Removed rather than moved into docs/. The rule says an explanation belongs in the relevant document, but pouring 1,700 lines of function-level commentary into the docset would bloat the thing readers actually use to answer questions about running a board. Nothing is lost that mattered: every removed line is in git history. Kept, because they are read by a tool rather than a person: biome-ignore suppressions, @ts-expect-error, compiler-read type annotations, triple-slash references, and turbopackIgnore in locate-wasm.ts, which changes how the bundler treats a dynamic read. Kept for a different reason: the six files a generated reference is built from, where the comment is the published page, and the comments inside create-meith's template literals, which are the scaffolded board's files rather than this codebase's code. Restored after the first pass took them: the GENERATED FILE banners on community.plugins.ts and packages/api/src/reference.ts. Their generators compare output byte for byte against what is on disk, so removing the banner failed board:gen:check — which is the gate doing its job. Verified: lint, all three typechecks, guards, guards:probe, and every generator and structural gate. All 37 biome-ignore and all 11 @ts-expect-error directives survive; the four that look lost to a grep were prose mentions inside removed comments.
…comments # Conflicts: # packages/create-meith/src/scaffold.test.ts
…eads The scanner shipped in #254 counted turbopackIgnore as an ordinary comment, so the hook would have refused any edit to packages/drivers/src/images/locate-wasm.ts — where that directive is what keeps Turbopack from following a dynamic read. Removing it to satisfy the guardrail would have changed how the file is bundled. The strip in this branch needed the same list and is what surfaced the gap: one comment survived a sweep that took 1,742 others, and the reason it survived was that it is not a comment in any sense that matters. Added alongside it: webpackIgnore and __PURE__, read by bundlers; @vitest-environment and @jsxImportSource, read by runtimes; and @deprecated, which TypeScript surfaces at every call site.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes MEI-143.
AGENTS.mdhas forbidden inline code comments since it was written. 1,743 had accumulated across 189 source files anyway, 1,281 of them written by agents, because nothing loaded the rule and nothing checked it. #254 fixed the second half. This is the backlog it left behind.1,743 → 1. 2,455 lines deleted across 187 files. The one survivor is a
turbopackIgnoredirective.Removed, not moved
MEI-143 was originally written to move each comment into the
docs/page that covers it, per AGENTS.md. That was overruled, and I think correctly: pouring 1,700 lines of function-level commentary into the docset would bloat the thing readers actually use to answer questions about running a board.Nothing that mattered is lost — every removed line is in
git log -p. Some were real "why" notes (theSameSite=Laxreasoning on the SSO cookie; why presence counts a guest only once the client has proved it keeps cookies; why a headline is split into blocks sotext-wrap: balanceruns per sentence). Recording that here so the trade-off is on the record.What was kept, and why
Read by a tool rather than a person:
biome-ignore,@ts-expect-error, compiler-read type annotations, triple-slash references, andturbopackIgnoreinlocate-wasm.ts— which changes how the bundler treats a dynamic read.The comment is the published page: the six files
pnpm theme:docsandpnpm plugin:docsbuild the theme-slot and plugin-hook references from.packages/theme-kit/src/view-models.tsalone is 739 lines of it.Not this codebase's code: the comments inside
create-meith's template literals are the scaffolded board's files. A comment orienting somebody in the project they just created is a different thing from a comment in ours.Two things the verification caught
// GENERATED FILE — do not edit.banners. Stripping them brokepnpm board:gen:check, because the generator compares its output byte for byte against what is on disk.community.plugins.ts(×2) andpackages/api/src/reference.tsare restored. The gate did its job.turbopackIgnoreas an ordinary comment, so the hook would have refused any edit tolocate-wasm.tsand pushed the next person toward deleting a load-bearing bundler directive. Fixed here, with a test, along withwebpackIgnore,__PURE__,@vitest-environment,@jsxImportSourceand@deprecated.The second is why this branch carries a
fix(agents):commit as well as the sweep: one comment surviving a pass that took 1,742 others is worth asking about, and the answer was a gap in the guardrail.Verification
pnpm lint,typecheck,typecheck:app,typecheck:site,guards,guards:probe,depcruise, and every generator and structural gate —workspace:check,root:check,release:check,i18n:check,slots:check,hooks:wired,board:gen:check,theme:docs:check,plugin:docs:check,perf:docs:check,api:docs:check,site:docs:check,marketplace:gen:check,board-installer:gen:check,vercel-template:gen:check,docs:index:check,docs:links:check,ci:parity:check.Directive counts checked before and after: all 37
biome-ignoreand all 11 real@ts-expect-errorsurvive. Four apparent@ts-expect-errorlosses were prose mentions inside removed comments, not directives.The unit suite is left to CI here. I started it three times locally and disturbed the tree under it twice (a
git stashwhile comparing counts, then mergingmainin); rather than report a run I had compromised, the honest thing is to let theStatic checksjob run it on a settled tree.Generated by Claude Code