test(agent-mode): codify and apply Claude backend test conventions (#2687)

This commit is contained in:
Zero Liu 2026-07-15 12:15:21 -04:00 committed by GitHub
parent 00b8e32685
commit 8bf8aec997
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 876 additions and 870 deletions

View file

@ -21,6 +21,10 @@ Copilot for Obsidian is an AI-powered assistant plugin that integrates various L
- **Always write generalizable solutions.** No hardcoded folder names, file patterns, or special-case logic (no "piano notes" / "daily notes" branches). Make varying behavior configurable, not hardcoded.
- **Never modify AI prompt content** — system prompts, model adapter prompts, etc. — unless the user explicitly asks.
- **Referential stability.** Never return a freshly-allocated `[]` / `{}` for an "empty" slice; return a frozen module-level constant (canonical examples: `EMPTY_PROVIDERS` / `EMPTY_CONFIGURED_MODELS` / `EMPTY_BACKENDS` in `src/settings/model.ts`).
- **Structure unit tests by module, class, and callable.** Use exactly one top-level `describe("moduleName", ...)` for the module under test; do not split the same subject across multiple top-level `describe` blocks. Within that module suite, wrap each class's tests in exactly one `describe("ClassName", ...)` so method ownership remains visible, then give each method exactly one nested `describe("methodName()", ...)` group. Keep module-level functions directly under the module suite, with exactly one `describe("functionName()", ...)` group per function. Merge cases that exercise the same callable. Separate same-callable groups only when a material test-lifecycle constraint makes merging misleading, and document that reason next to the groups. Write `it(...)` descriptions that state the observable behavior without requiring the reader to inspect the test body.
- **Pair every production TypeScript function and method with unit coverage.** Directly test exported and public callables; cover private and module-local helpers through their observable public contract unless direct isolation materially improves clarity. Test-only helper functions are exempt.
- **Document exported functions and public methods of exported classes when their purpose, contract, or parameters are not self-evident.** Simple functions and methods with unambiguous names and parameters may omit JSDoc. When JSDoc is needed, explain why the callable exists and the goal it serves without repeating its implementation, and add an `@param` entry for every parameter that explains its meaning without repeating its TypeScript type.
- **Document every exported class with JSDoc.** State what the class is responsible for managing and where its boundary ends so readers can understand its duty without reading the implementation.
- **Never use `console.log`** — use `logInfo()` / `logWarn()` / `logError()` from `@/logger`.
- **Comment the why, not the what;** minimal comments, no milestone/plan-step refs. → [`STYLE_GUIDE.md`](./designdocs/agents/STYLE_GUIDE.md)
- **Never edit `styles.css`** (generated); edit `src/styles/tailwind.css`, no inline `style`, no arbitrary font sizes, wrap class strings in `cn()`. → [`STYLE_GUIDE.md`](./designdocs/agents/STYLE_GUIDE.md)
@ -113,11 +117,12 @@ gh api -X POST repos/OWNER/REPO/pulls/PR/comments/ROOT_COMMENT_ID/replies -f bod
```
Never open a review draft: `POST /pulls/PR/reviews` without an `event` (and the
UI's "Start a review") leaves the reply *pending*, which means invisible — it
UI's "Start a review") leaves the reply _pending_, which means invisible — it
never reaches the reviewer, it is absent from the comments API, and its thread
still reads as unanswered. Before finishing, confirm none exists:
```bash
gh api repos/OWNER/REPO/pulls/PR/reviews --jq '[.[]|select(.state=="PENDING")]|length' # must be 0
```
<!-- brevilabs-review-guidelines:end -->

View file

@ -15,6 +15,11 @@ language, comment, styling, and code-structure rules.
- Custom hooks for reusable logic
- Props interfaces defined above components
- Prefer `useSyncExternalStore` for mutable external sources that expose a
snapshot and subscription. Do not subscribe and increment dummy state solely
to force a render. Snapshots must remain referentially stable while their
semantic value is unchanged; continue to use `useState` for component-owned
UI state.
## Comments
@ -24,14 +29,21 @@ carry the **why** — the things a reader cannot recover by reading the code.
- **Comment the why, not the what.** Document non-obvious constraints,
invariants, gotchas, and "why this exists / why not the obvious alternative".
If a comment only restates what the next line plainly says, delete it.
- **Default to minimal comments — JSDoc is not required on every function.** A
function with a clear name and signature needs no doc block. Add one only when
there's a why worth recording. When you do write a "what", keep it to one
short line.
- **Drop redundant `@param`/`@returns`.** Keep a tag only when it adds
information the type and name don't already convey (e.g. "`null` means the
agent is CLI-managed, so no key is stored"). Don't write a `@param` line that
just echoes the parameter name and type.
- **Document exported functions and public methods of exported classes when the
contract is not self-evident.** JSDoc is optional for a simple callable whose
purpose and parameters are already unambiguous. When JSDoc is needed, explain
why the callable exists and the goal it serves without narrating its concrete
implementation. Internal functions and non-public methods still default to
no doc block unless they carry a non-obvious constraint or invariant.
- **Document every parameter in an included JSDoc block by meaning, not type.**
Include one `@param` tag per parameter and explain its role or relevant
semantics. TypeScript owns the type information, so never repeat it in
JSDoc. Add `@returns` only when the return value has semantics the signature
cannot express.
- **Document every exported class with JSDoc.** Describe the state or lifecycle
the class owns, the responsibility it coordinates, and the boundary it does
not cross. The goal is to make the class's duty clear without requiring a
reader to inspect its methods or private fields.
- **No milestone or plan-step references in code.** Never write `M1`/`M3`,
`§4.3`, "step 3 of the plan", "after milestone X lands", or similar. These are
scaffolding for whoever is _writing_ a branch and are meaningless to whoever _reviews or maintains_ the code later.

File diff suppressed because it is too large Load diff