Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .claude/rules/code-style.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,8 @@ The counts, thresholds, and carve-outs behind these live in the skill. What foll
- **A component gets a directory, not a loose file.** Every one lives in its own kebab-case directory under `src/components/` with a PascalCase file and a colocated test, as [`navbar/Navbar.tsx`](../../src/components/navbar/Navbar.tsx) does. Six of the seven directories spell the name that way, and [`Stars/`](../../src/components/Stars/StarsBackground.tsx) is the single PascalCase exception rather than a second convention: match the six. Related files are grouped into a subdirectory rather than left flat beside unrelated ones, and entries sharing a name prefix are the group to propose.
- **Count the files sitting directly in a directory**, whatever subdirectories sit beside them: one subdirectory does not make the loose files next to it grouped. [`src/components/ServiceWorkerRegister.tsx`](../../src/components/ServiceWorkerRegister.tsx) and the two `ThemeRegistry` files sit directly in `src/components/` beside seven component directories, so the count there is three rather than ten.
- **A setting the tooling reads from configuration is set once, never per file.** [`jest.config.js`](../../jest.config.js) already sets `testEnvironment: 'jsdom'` for every test, so no test file carries a `@jest-environment` docblock. Path aliases are declared in [`tsconfig.json`](../../tsconfig.json) and mirrored in [`jest.config.js`](../../jest.config.js) rather than re-declared per import. Where the same directive would go into three or more files, **search for the key rather than for the directive's own spelling**, since the two are rarely the same word, and hoist the majority while leaving the minority declared. Moving a directive into the key the tool reads is not deleting it.
- **Reuse before writing.** Check this repository's own [`helpers`](../../src/helpers/ascii.ts) and [`util`](../../src/util/cookieConsent.ts) modules, then [`package.json`](../../package.json), then the platform, before hand-writing behaviour that has a name outside this repository. Where nothing present provides it, say so rather than adding a dependency. Never hand-roll anything that signs, verifies, hashes a credential, or settles an authorization outcome.
- **A utility does one thing, and a parameter it branches on is a second thing.** Count the parameters of every function you write or review, splitting those supplying data from those the body branches on rather than operates on. Every exported helper here already matches that shape: [`convertAAAAHH(aaaaahhhh: string)`](../../src/helpers/aaaahhhh.ts) takes one value and no switch, and [`hasCookieConsent()`](../../src/util/cookieConsent.ts) takes none, so a new helper carrying two booleans is the outlier rather than the norm. More than one switch on a utility is the trigger to split it into one function per behaviour; a function coordinating a sequence takes its modes legitimately. Each function a split produces has one caller the day it lands, and that is what a split looks like rather than a reason to keep the flag.
- **Reuse before writing.** Check this repository's own [`helpers`](../../src/helpers/ascii.ts) and [`util`](../../src/util/cookieConsent.ts) modules, then [`package.json`](../../package.json) with the lockfile beside it, then the platform, before hand-writing behaviour that has a name outside this repository. Read what the modules a file already imports export before accepting a hand-written block beneath them. Where nothing present provides it, say so rather than adding a dependency. Never hand-roll anything that signs, verifies, hashes a credential, or settles an authorization outcome.

## TypeScript

Expand Down
102 changes: 65 additions & 37 deletions .claude/skills/audit-pr/SKILL.md

Large diffs are not rendered by default.

7 changes: 4 additions & 3 deletions .claude/skills/audit-pr/agents/finding-refuter.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ Search the added lines of the diff for the quote as a literal string, before sea

A `[REDACTED]` placeholder is the one exception, and it narrows the search rather than skipping it. Search the added lines for the text around the placeholder, which is every part of the quote except the credential value, and never for the value itself. Confirm that one added line carries all of that surrounding text in the order the quote gives it, then record which parts matched. A redacted quote whose surrounding text matches no added line fails this question exactly as any other quote would.

**A structural finding carries a count instead of a quote, and it is checked by counting again.** Its defect is the shape of the code rather than any line of it, so no string can be matched: nothing in a file says the directory holds forty files or the type carries twenty members. Re-derive the number the finding states, by listing the directory and counting only the files sitting directly in it, reading the member list, measuring the file, finding each occurrence of the repeated block, or searching the added lines for the repeated declaration, and compare it against what the finding claimed. A finding about a repeated declaration is measured against the configuration key that would carry it once, so check that the finding names that key and that the key exists. Treat this question as passed where the count holds and the finding also states what the count is measured against, whether that is the sibling directories, the neighbouring files, or the callers touching four of twenty members. A count that no longer holds fails exactly as a missing quote does. A finding stating a number with nothing to compare it against fails too, since a bare number is a fact about the code rather than a claim about it, and there is nothing for this question to check.
**A structural finding carries a count instead of a quote, and it is checked by counting again.** Its defect is the shape of the code rather than any line of it, so no string can be matched: nothing in a file says the directory holds forty files or the type carries twenty members. Re-derive the number the finding states, by listing the directory and counting only the files sitting directly in it, reading the member list, measuring the file, finding each occurrence of the repeated block, re-reading the signature and splitting its parameters into data and switches, or searching the added lines for the repeated declaration, and compare it against what the finding claimed. A finding about a repeated declaration is measured against the configuration key that would carry it once, so check that the finding names that key and that the key exists. Treat this question as passed where the count holds and the finding also states what the count is measured against, whether that is the sibling directories, the neighbouring files, or the callers touching four of twenty members. A count that no longer holds fails exactly as a missing quote does. A finding stating a number with nothing to compare it against fails too, since a bare number is a fact about the code rather than a claim about it, and there is nothing for this question to check.

## Trace the mechanism the finding asserts

Expand Down Expand Up @@ -75,7 +75,7 @@ Reconstruct the before-state from the removed lines in the same hunk, or from th
- The defect holds only after the change: question passed.
- The defect holds before and after, and the change is what makes it reachable or wrong: question passed, and the finding states which part is pre-existing.
- The defect holds before and after with the same effect: PRE-EXISTING, with the before-state line quoted.
- The finding is structural and its count moved: question passed. A file this change leaves longer, a type it leaves wider, and a directory it leaves fuller are what this diff produced, whatever their size beforehand, so re-derive the before-count from the base revision and pass the question on the difference. Only a count this change did not move is PRE-EXISTING.
- The finding is structural and its count moved: question passed. A file this change leaves longer, a type it leaves wider, a signature it leaves carrying another switch, and a directory it leaves fuller are what this diff produced, whatever their size beforehand, so re-derive the before-count from the base revision and pass the question on the difference. Only a count this change did not move is PRE-EXISTING.

PRE-EXISTING is not a gentler REFUTED. It says the claim is true and this diff is the wrong place to charge it. REFUTED says the claim does not hold.

Expand All @@ -90,14 +90,15 @@ A fix whose correctness follows from reading code is settled by reading it, and
- Shell quoting: a bare variable against a quoted one, where the value contains a space or a glob character.
- Trigger filters: whether a filter listing `docs/**` fires for `docs/index.md`, for `docs/api/spec.md`, and for a file at the repository root.

**This agent does not run a tool to settle one of those.** Naming the dependency is the answer, and the caller decides whether one run for the whole review is worth it. This question has four outcomes, and only the last two touch the verdict.
**This agent does not run a tool to settle one of those.** Naming the dependency is the answer, and the caller decides whether one run for the whole review is worth it. This question has eight outcomes. Most of them decide the fix alone, leaving the finding standing; only the last can turn the verdict to REFUTED.

- Read code that settles it, and the fix works: passed.
- Correctness depends on tool behaviour from the list above, or on executing code out of the change: passed, and the finding ships with the fix marked `unverified fix`, naming what would confirm it.
- The fix proposes an abstraction and the abstraction is premature: the fix is deleted and the finding survives on its observation alone. Generalizing costs more than the duplication it removes wherever the copies would change for different reasons, so a fix leaving an abstraction with a single caller, a generic parameter with a single instantiation, or configuration nobody would set fails here. **This outcome never refutes a duplication finding.** The occurrences were counted and they are real; what failed is one proposal for what to do about them, and the caller keeps the observation with its paths for a human to weigh.
- The fix sets a key the project's own tool already defines: passed, and the premature-abstraction outcome above does not reach it. **Configuration nobody would set means a key the fix invents.** A key the tool already defines, which files in the tree are already setting one at a time, is the opposite, since setting it once at the level the tool reads it removes configuration rather than adding it. Open the tool's configuration and look for the key before deciding, searching for the key rather than for the per-file directive's own spelling, because the two are rarely the same word. Then ask who else the new default governs: a default that changes behaviour for files outside the change fails here unless the fix leaves those files declared.
- The fix replaces written code with a call to something already present: passed, and the premature-abstraction outcome does not reach it either, since reusing an existing implementation removes an abstraction rather than adding one. What this question asks instead is whether the named module, package, or standard-library symbol resolves at the version the manifest pins, and whether its surface covers the case the block handles. Name the manifest or lockfile you opened.
- The fix proposes a grouping and a named group would hold one file: that is a rename, and the fix is deleted while the count behind it survives. A grouping passes where every group it names holds two or more of the files counted.
- The fix splits one unit into narrower units: passed, and the premature-abstraction outcome does not reach it, since decomposition removes a responsibility rather than adding an abstraction. **Every unit a split produces has one caller on the day it lands**, which is what a split looks like rather than evidence against it, so counting callers refutes nothing here. What this question asks instead is whether each resulting unit has one reason to change, and whether the caller that made one call now reads as a sequence of named steps. A split that leaves the same branching behind a new name fails.
- Reading shows the fix changes nothing: the fix is deleted. The finding survives if the claim stands without a fix; otherwise the verdict is REFUTED.

## Verdict format and the disposition of a refuted finding
Expand Down
Loading