Skip to content

Commit a32fc04

Browse files
theCodeDriftclaude
andcommitted
style: widen the house-style rules to CLAUDE.md and .conventions
First slice of the broadening in #169. The three prose rules were scoped to READMEs only, because landing them repo-wide meant roughly 2300 findings at level: error. This adds the two smallest surfaces and fixes what they catch, so the slice is green on its own. 41 findings: 39 em dashes rewritten as a period, comma, colon, or parentheses per the rule's own message rather than swapped mechanically for one substitute, and two uses of "simply" where the sentence was describing a real distinction ("merely lives further down", "just out of date") rather than hedging. Fenced code blocks are untouched. These rules carry Vale's default scope, which does not read them, so the em dashes in the shell comments at CLAUDE.md:155 and STYLEGUIDE-CODE.md:222 are out of scope and stay. Remaining surfaces, in the order #169 proposes: TypeScript comments (Vale's comments-only tier), then the agent-facing recipe text, which is the largest. openspec/changes/archive/ gets a permanent exclusion rather than a slice. Refs #169 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017cEN93Acyp4zBwP3oDnyy1
1 parent 54142d7 commit a32fc04

5 files changed

Lines changed: 75 additions & 45 deletions

File tree

‎.conventions/STYLEGUIDE-CODE.md‎

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,7 @@ interface GitHubComment {
124124

125125
### Export Types Referenced by Public API Signatures
126126

127-
**DO NOT** remove `export` from types that are transitively referenced by exported functions, values, or other exported types — even if tools like knip report them as "unused exports." With `declaration: true` in `tsconfig`, TypeScript requires all types in exported signatures to be exported themselves.
127+
**DO NOT** remove `export` from types that are transitively referenced by exported functions, values, or other exported types, even if tools like knip report them as "unused exports." With `declaration: true` in `tsconfig`, TypeScript requires all types in exported signatures to be exported themselves.
128128

129129
Before removing an `export` from a type, check whether any exported function or value references it in its signature (parameters, return types, or fields of other exported types).
130130

@@ -150,7 +150,7 @@ interface LayerResult { ... } // breaks declaration emit for VerifyResult
150150

151151
- Knip tracks direct import usage, not transitive type reachability through exported signatures
152152
- Removing these exports causes `declaration: true` to fail with "exported function has or is using private name" errors
153-
- The fix is tedious — each type must be re-exported individually, often across multiple review cycles
153+
- The fix is tedious: each type must be re-exported individually, often across multiple review cycles
154154

155155
## Cross-Worker Durable Object Access
156156

@@ -201,7 +201,7 @@ import type { UserDO, GitHubOrganizationDO } from "@taskless/storage";
201201

202202
### Verify Build Output In The Build, Not By Parsing It
203203

204-
**A failing build is still a valid test — of the build.** When an invariant is about a build artifact, enforce it where the artifact is produced. If a bundle must not contain something, the build should refuse to emit it, rather than emitting it and leaving a test to go looking afterwards. An invariant enforced at production time cannot be violated; one enforced afterwards can only be detected.
204+
**A failing build is still a valid test of the build.** When an invariant is about a build artifact, enforce it where the artifact is produced. If a bundle must not contain something, the build should refuse to emit it, rather than emitting it and leaving a test to go looking afterwards. An invariant enforced at production time cannot be violated; one enforced afterwards can only be detected.
205205

206206
**DO NOT** reconstruct a fact about generated output by parsing that output.
207207

@@ -240,7 +240,7 @@ for (const specifier of specifiers) {
240240
}
241241
```
242242

243-
**Tests that _use_ a built artifact are fine.** Importing the built entry and asserting on its behavior, or spawning the built CLI and asserting on its output, are ordinary tests. The rule is not "tests must not touch build output" — it is that tests must not re-derive what the build already knew.
243+
**Tests that _use_ a built artifact are fine.** Importing the built entry and asserting on its behavior, or spawning the built CLI and asserting on its output, are ordinary tests. The rule is not "tests must not touch build output". It is that tests must not re-derive what the build already knew.
244244

245245
```typescript
246246
// ✅ Fine - uses the artifact, asserts on behavior
@@ -252,27 +252,27 @@ const { stdout } = await execFileAsync("node", [builtCli, "help"]);
252252
expect(stdout).toContain("Usage:");
253253
```
254254

255-
**Do not add a dependency in order to test an assertion.** If a test needs a parser to make sense of an artifact, that is the signal the check is in the wrong place — the generator already has the structured data. Reach for a new devDependency only when several tests need it and nothing in the existing toolchain can answer the question.
255+
**Do not add a dependency in order to test an assertion.** If a test needs a parser to make sense of an artifact, that is the signal the check is in the wrong place: the generator already has the structured data. Reach for a new devDependency only when several tests need it and nothing in the existing toolchain can answer the question.
256256

257-
**Worked example.** `packages/cli/test/prompts.test.ts` asserted that the built `dist/prompts.js` chunk graph never reaches the CLI entry or a host capability, by regex-scanning the built JavaScript for `from "…"` to reconstruct the import graph. A built chunk embeds every help recipe as a string literal, and the `engine-selection` recipe contains the phrase `a different axis from "which engine"` — so the scan reported `dist/prompts.js graph imports which engine`. Prose was read as an import.
257+
**Worked example.** `packages/cli/test/prompts.test.ts` asserted that the built `dist/prompts.js` chunk graph never reaches the CLI entry or a host capability, by regex-scanning the built JavaScript for `from "…"` to reconstruct the import graph. A built chunk embeds every help recipe as a string literal, and the `engine-selection` recipe contains the phrase `a different axis from "which engine"`, so the scan reported `dist/prompts.js graph imports which engine`. Prose was read as an import.
258258

259259
The fixes that did not work, and why:
260260

261-
| Attempt | Why it was rejected |
262-
| ---------------------------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
263-
| Filter candidates by specifier shape (`/^(?:node:)?[@\w./-]+$/`) | Passed only because that phrase contains a space. Measured against the real bundle the regex yields `["which engine"]` and the filter drops it — but `differs from "static-tier"` is a bare hyphenated name with no whitespace and would have been reported. The guard held by luck of punctuation. |
264-
| Add `es-module-lexer` as a devDependency | Parsed the graph correctly, but bought a dependency — and a second major version, since vite already pulls 1.7.0 transitively — to serve a single test. |
265-
| Anchor the regex to line-start | Matched the lexer exactly on today's bundles, but required `from` on the same line as `import`. A future bundler that wrapped a long import would silently stop detecting real imports — trading a loud false positive for a quiet false negative in the guard whose entire job is catching a leak. |
261+
| Attempt | Why it was rejected |
262+
| ---------------------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
263+
| Filter candidates by specifier shape (`/^(?:node:)?[@\w./-]+$/`) | Passed only because that phrase contains a space. Measured against the real bundle the regex yields `["which engine"]` and the filter drops it, but `differs from "static-tier"` is a bare hyphenated name with no whitespace and would have been reported. The guard held by luck of punctuation. |
264+
| Add `es-module-lexer` as a devDependency | Parsed the graph correctly, but bought a dependency, and a second major version since vite already pulls 1.7.0 transitively, to serve a single test. |
265+
| Anchor the regex to line-start | Matched the lexer exactly on today's bundles, but required `from` on the same line as `import`. A future bundler that wrapped a long import would silently stop detecting real imports, trading a loud false positive for a quiet false negative in the guard whose entire job is catching a leak. |
266266

267-
The resolution: rollup's `OutputChunk` already exposes `imports` and `dynamicImports` — the exact resolved graph. The check moved into a vite plugin that fails the build, and the test was deleted.
267+
The resolution: rollup's `OutputChunk` already exposes `imports` and `dynamicImports`, the exact resolved graph. The check moved into a vite plugin that fails the build, and the test was deleted.
268268

269269
The same reasoning forbids adding a YAML parser to assert on generated config, or an HTML parser to assert on rendered output. In each case the generator knows the answer and the test is guessing at it.
270270

271271
**Rationale:**
272272

273273
- An invariant enforced at production time cannot be violated; one enforced afterwards can only be detected
274274
- Parsing generated text reconstructs information the generator already had, using a weaker tool
275-
- A check that needs a parser is a check in the wrong place — move it to where the structured data lives
275+
- A check that needs a parser is a check in the wrong place; move it to where the structured data lives
276276
- A build that fails is a faster, earlier signal than a test that fails, and it cannot be skipped
277277
- Regexes over generated output are brittle in the worst direction: they break on content that merely resembles code, and they quietly stop matching when the generator's formatting changes
278278

‎.taskless/rules/vale/no-blocklist-phrases/.vale.ini‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,18 @@ tskl) rule = no-blocklist-phrases
55
BasedOnStyles =
66
no-blocklist-phrases.no-blocklist-phrases = YES
77

8+
# Agent-facing instructions and the house conventions. Read as often as the
9+
# READMEs are, by both people and agents, and small enough to keep conforming.
10+
[CLAUDE.md]
11+
tskl) rule = no-blocklist-phrases
12+
BasedOnStyles =
13+
no-blocklist-phrases.no-blocklist-phrases = YES
14+
15+
[.conventions/*.md]
16+
tskl) rule = no-blocklist-phrases
17+
BasedOnStyles =
18+
no-blocklist-phrases.no-blocklist-phrases = YES
19+
820
# Test fixtures are inputs to the CLI's own suite, not documentation.
921
[**/test/fixtures/**/README.md]
1022
tskl) rule = no-blocklist-phrases

‎.taskless/rules/vale/no-em-dashes/.vale.ini‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,18 @@ tskl) rule = no-em-dashes
55
BasedOnStyles =
66
no-em-dashes.no-em-dashes = YES
77

8+
# Agent-facing instructions and the house conventions. Read as often as the
9+
# READMEs are, by both people and agents, and small enough to keep conforming.
10+
[CLAUDE.md]
11+
tskl) rule = no-em-dashes
12+
BasedOnStyles =
13+
no-em-dashes.no-em-dashes = YES
14+
15+
[.conventions/*.md]
16+
tskl) rule = no-em-dashes
17+
BasedOnStyles =
18+
no-em-dashes.no-em-dashes = YES
19+
820
# Test fixtures are inputs to the CLI's own suite, not documentation.
921
[**/test/fixtures/**/README.md]
1022
tskl) rule = no-em-dashes

‎.taskless/rules/vale/no-hedging/.vale.ini‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,11 @@ tskl) rule = no-hedging
55
BasedOnStyles =
66
no-hedging.no-hedging = YES
77

8+
[CLAUDE.md]
9+
tskl) rule = no-hedging
10+
BasedOnStyles =
11+
no-hedging.no-hedging = YES
12+
813
[.conventions/*.md]
914
tskl) rule = no-hedging
1015
BasedOnStyles =

0 commit comments

Comments
 (0)