diff --git a/.claude/skills/enhance-claude-docs/SKILL.md b/.claude/skills/enhance-claude-docs/SKILL.md index f9cff986d..6abf9c459 100644 --- a/.claude/skills/enhance-claude-docs/SKILL.md +++ b/.claude/skills/enhance-claude-docs/SKILL.md @@ -136,8 +136,9 @@ For each thread that passes Stage 1, determine if it contains a documentation-wo | Signal | Target file | |--------|-------------| -| Reviewer correcting a pattern violation not yet in rules | `agent_docs/rules.md` | +| Reviewer correcting a pattern violation or convention (including anti-patterns) | `agent_docs/conventions.md` (add inline **NEVER** next to the relevant convention) | | Explaining WHY a convention exists or should be followed | `agent_docs/conventions.md` | +| New testing guidelines or quality rules | `agent_docs/rules.md` (testing & quality only) | | New project structure info, service patterns, or architecture decisions | `agent_docs/architecture.md` | | New commands, quick-reference items, or workflow changes | `Agents.md` | | Top-level pointers or high-level project changes (rare) | `CLAUDE.md` | @@ -170,8 +171,8 @@ If actionable insights exist: 1. For each insight, determine the correct file and section. 2. Edit the files using the Edit tool. Follow existing formatting, heading levels, and conventions in each file. -3. For `agent_docs/rules.md`, add new items under the appropriate "NEVER" subsection or create a new subsection if needed. Follow the existing pattern: bold "NEVER" + action, followed by explanation with rationale. -4. For `agent_docs/conventions.md`, add to the relevant section or create a new subsection following existing patterns. +3. For `agent_docs/conventions.md`, add conventions and anti-patterns to the relevant section. Anti-patterns go inline as **NEVER** next to the related convention — only when they add a non-obvious *why* or call out a specific trap. Do not add NEVERs that merely restate the convention in negative form. +4. For `agent_docs/rules.md`, add only testing guidelines, integration test rules, documentation/JSDoc rules, or quality checklist items. This file does not contain coding conventions or anti-patterns — those belong in `conventions.md`. 5. For `agent_docs/architecture.md`, update tables or sections as appropriate. 6. For `Agents.md`, update the quick reference or add new sections. diff --git a/agent_docs/conventions.md b/agent_docs/conventions.md index b91d56f57..91a949524 100644 --- a/agent_docs/conventions.md +++ b/agent_docs/conventions.md @@ -7,8 +7,11 @@ - **UPPER_SNAKE_CASE**: constants (`DEFAULT_PAGE_SIZE`, `TASK_ENDPOINTS`) - **File names**: kebab-case for general files (`api-client.ts`), dot-separated for type/model files (`tasks.types.ts`, `tasks.models.ts`) - Prefer `private` keyword over underscore prefix for private methods -- No `any` type — use `unknown` if truly unknown, then validate -- Mark optional fields as optional in type interfaces +- No `any` type — use `unknown` if truly unknown, then validate. +- **NEVER** use `as unknown as` type casts — refactor to make types flow naturally. Casts hide real type errors and break when upstream types change. +- Mark optional fields as optional in type interfaces — over-requiring causes runtime `undefined` access on fields the API didn't return. +- Use enums for fixed value sets — **NEVER** leave raw strings/numbers. Raw values lose type safety and autocomplete. +- **NEVER** write `param || {}` for required parameters — this hides bugs by silently accepting missing required data at call sites. ## General conventions @@ -16,8 +19,9 @@ - Types live in `src/models/{domain}/{domain}.types.ts`. Internal-only types go in `*.internal-types.ts`. - Constants live in `src/utils/constants/`. Endpoints are split per domain in `src/utils/constants/endpoints/` (e.g., `data-fabric.ts`, `maestro.ts`, `orchestrator.ts`). - Subpath exports: when adding a new service module, add entries to `package.json` `exports` and `rollup.config.js`. -- Every public service method must be decorated with `@track('ServiceName.MethodName')` for telemetry. +- Every public service method must be decorated with `@track('ServiceName.MethodName')` for telemetry — gaps are invisible until production debugging, when they're expensive. - Use named imports/exports (avoid default exports). Use barrel exports (`index.ts`) for public API. Never export internal types from barrel exports. +- **Barrel files must use `export * from`**, not `export type * from`. Using `export type` re-exports only type declarations and silently drops runtime values (class constructors, enums), causing `undefined` errors for SDK consumers who import them. ## Type naming @@ -26,10 +30,12 @@ - **Final response type**: `type {Entity}GetResponse = Raw{Entity}GetResponse & {Entity}Methods` — defined in `*.models.ts`, combining raw data with bound methods. - **Options types**: `{Entity}GetAllOptions`, `{Entity}GetByIdOptions`, `{Entity}{Operation}Options` (e.g., `TaskAssignmentOptions`, `ProcessInstanceOperationOptions`). Compose with `RequestOptions & PaginationOptions & { ... }` for list methods. - **Common base types**: `BaseOptions` (expand, select), `RequestOptions` (extends BaseOptions with filter, orderby), `OperationResponse` (success + data) — all from `src/models/common/types.ts`. -- **Use "Options" not "Request"** for parameter types — never `{Entity}{Operation}Request`. +- **Use "Options" not "Request"** for parameter types — the entire SDK uses `{Entity}{Operation}Options`. - **Required parameters are always positional; Options objects are reserved for optional parameters only.** Required values (IDs, keys, data) are positional arguments. Options objects are always the last parameter, always marked `?`, and contain only optional fields. E.g., `getOutput(jobKey: string)` not `getOutput(options: { jobKey: string })`, `close(instanceId, folderKey, options?)` not `close(options: { instanceId, folderKey })`. +- **NEVER** duplicate fields across option types — extend existing ones. If `CaseInstanceOperationOptions` already has `comment`, extend it instead of re-declaring. When the shape is identical, use `extends` (e.g., `export interface EntityUpdateRecordByIdOptions extends EntityGetRecordByIdOptions {}`). +- **NEVER** use type aliases for response types — even when the shape matches an existing one, use an `extends` interface. Type aliases (e.g., `export type EntityUpdateRecordResponse = EntityRecord`) break TypeDoc docs generation by not rendering as standalone types. Use `export interface EntityUpdateRecordResponse extends EntityRecord {}` instead. -Method names: **singular** for single-item ops (`insertRecordById`), **plural** for batch (`insertRecordsById`). Prefer plurals over `batch` prefix. +Method names: **singular** for single-item ops (`insertRecordById`), **plural** for batch (`insertRecordsById`). **NEVER** use `batch` prefix — the SDK convention is singular/plural to distinguish cardinality. **Singular vs batch patterns:** @@ -52,15 +58,17 @@ The method attachment pattern: - `create{Entity}WithMethods(rawData, service)` — merges raw data + methods via `Object.assign({}, rawData, methods)` - Methods validate required fields (`if (!data.id) throw new Error(...)`) before delegating +**Include output/attachment indicator fields in response types:** When an entity response can reference a file or attachment (e.g., `outputFile` on a Job indicating a blob attachment key), include that field in `Raw{Entity}GetResponse` even if the primary retrieval method does not fetch the file. This lets callers check for output availability without an extra API call. + ## Method attachment (when to bind methods to response objects) - **Not every service method gets bound.** Only bind methods that operate ON a specific entity after retrieval — state-changing operations (assign, cancel, complete, insert, update, delete) and contextual reads that need the entity's ID. -- **Never bind**: `getAll()`, `getById()`, `create()`, and cross-entity queries like `getUsers()`. +- **NEVER** bind `getAll()`, `getById()`, `create()`, or cross-entity queries — these are service-level entry points. Binding them creates circular nonsense (an entity that retrieves itself). - **Read-only services don't bind at all** — Assets, Buckets, Queues, Processes, ChoiceSets, Cases, and ProcessIncidents have no `{Entity}Methods` interface. ## Response transformation pipeline -Transform functions live in `src/utils/transform.ts`. Not every service uses every step — inspect the actual API response to decide which are needed. +Transform functions live in `src/utils/transform.ts`. Not every service uses every step — inspect the actual API response to decide which are needed. Each step must be justified by what the API actually sends. Never return raw API responses — apply all applicable pipeline steps. **Available steps (apply in this order, skip any that don't apply):** @@ -81,7 +89,9 @@ Transform functions live in `src/utils/transform.ts`. Not every service uses eve **Outbound requests** (SDK → API): use `transformRequest(data, {Entity}Map)` (auto-reverses field map) and `camelToPascalCaseKeys()`. -**Field maps vs case conversion:** `{Entity}Map` is for semantic renames only. Case conversion is handled by `pascalToCamelCaseKeys()`. Do not add case-only entries to a field map. +**Field maps vs case conversion:** `{Entity}Map` is for semantic renames only. Case conversion is handled by `pascalToCamelCaseKeys()`. **NEVER** add case-only entries to a field map — mixing them causes double-conversion bugs. + +**Data Fabric exception:** Do NOT apply `pascalToCamelCaseKeys()` or any field-rename transforms to Data Fabric entity record data (`EntityRecord`, record fields returned by `getRecordById`, `getAllRecords`, etc.). DF entity field names are user-defined schema columns and must be returned exactly as the API sends them — casing is part of the schema contract. Only system-generated DF fields (e.g., `Id`, `CreatedBy`) use PascalCase, and those are also left untransformed to keep behavior consistent. ## Endpoint constants @@ -91,8 +101,9 @@ Defined in `src/utils/constants/endpoints/` with separate files per domain (e.g. - Parameterized endpoints: arrow functions — `GET_BY_ID: (id: string) => '${BASE}/api/v1/instances/${id}'` - Operation endpoints: `CANCEL: (id: string) => '${BASE}/api/v1/instances/${id}/cancel'` - All objects use `as const` -- **Group nested endpoints logically** (e.g., `ENTITY.ATTACHMENT.DOWNLOAD` not flat). -- **Use consistent param names** across endpoints. Avoid redundancy. +- **Group nested endpoints logically** (e.g., `ENTITY.ATTACHMENT.DOWNLOAD` not flat). Use short names — under a `CASE` group, use `REOPEN` not `REOPEN_CASE` since the group context already provides the prefix. +- **Use consistent param names** across endpoints in the same group — if one endpoint uses `instanceId`, all should. +- **NEVER** copy-paste JSDoc comments between endpoint groups — each constant needs its own comment. A "Asset Service Endpoints" comment on `JOB_ENDPOINTS` is a review rejection. ## Pagination @@ -124,6 +135,7 @@ Naming: `{SERVICE}_PAGINATION` for response shape, `{SERVICE}_OFFSET_PARAMS` or ## Export naming +- **NEVER** use internal product/team names in service file paths or class names — use the user-facing domain name instead. E.g., name the file `feedback.ts` under `conversational-agent/feedback/`, not `llmops.ts` under `llmops/`. Internal product names change and leak internal team organization into the public API. - Internal class: `{Entity}Service` (e.g., `EntityService`, `TaskService`) - Public alias in `index.ts`: plural noun (e.g., `EntityService as Entities`, `TaskService as Tasks`) - Legacy services export both names for backward compatibility @@ -137,6 +149,8 @@ Types in `{domain}.types.ts` are public (re-exported through barrel). Types in ` **Put in `types.ts`:** All types in public method signatures, `Raw{Entity}GetResponse` types that users compose with `{Entity}Methods`. +**NEVER** re-export `internal-types.ts` through `index.ts` — re-exporting pollutes the public API and creates breaking-change risk when internal formats change. + ## OData prefix pattern OData APIs require `$` prefix on query params. The SDK accepts clean camelCase keys and adds the prefix via `addPrefixToKeys()` from `src/utils/transform.ts`. @@ -168,13 +182,14 @@ constructor(instance: IUiPath) { } ``` -If the constructor only calls `super()` with no additional setup, omit it entirely (redundant constructor rule). +If the constructor only calls `super()` with no additional setup, omit it entirely. ## Error types -- **`ValidationError`** — for **user input validation only**: missing required params, invalid option values, malformed user-provided data. Example: `if (!jobKey) throw new ValidationError(...)`. +- **`ValidationError`** — for **user input validation only**: missing required params, invalid option values, malformed user-provided data. Example: `if (!jobKey) throw new ValidationError(...)`. **NEVER** use for server-side issues like failed JSON parsing — use `ServerError` instead. Misusing it misrepresents the error source. - **`ServerError`** — for server-side issues: failed JSON parsing of API responses, unexpected response formats, API returning unparseable data. Example: `catch { throw new ServerError({ message: 'Failed to parse output as JSON' }) }`. - **`ErrorFactory.createFromHttpStatus()`** — for HTTP error responses from external calls (blob downloads, etc.). Maps status codes to typed errors automatically. +- **NEVER** add unnecessary type casts on already-typed values — if `blobAccess.headers` is already `Record`, use a simple spread `{ ...blobAccess.headers }` instead of `arrayDictionaryToRecord()` with `as unknown as` casts. ## BaseService @@ -212,3 +227,9 @@ interface OperationResponse { success: boolean; data: TData; } **Use for:** Lifecycle operations (cancel, pause, resume), bulk operations with error checking via `processODataArrayResponse()`. **DO NOT use for:** `getAll()`, `getById()`, `create()`, methods returning entity data directly. + +## Code hygiene + +- **NEVER** leave unused code — unused imports, variables, redundant constructors that only call `super()`. Linter (oxlint) catches these. +- **NEVER** commit sensitive files — `.env`, `credentials.json`, `*.key`, `*.pem`, hardcoded API keys/tokens. +- **NEVER** define static lookup tables inside method bodies — move them to module-level constants or `*.internal-types.ts`. A static mapping that doesn't change between calls (e.g., `TaskTypeEndpoints`) rebuilt on every invocation wastes memory and hides structure. diff --git a/agent_docs/rules.md b/agent_docs/rules.md index 837a616d0..cb3db20f0 100644 --- a/agent_docs/rules.md +++ b/agent_docs/rules.md @@ -1,86 +1,22 @@ -# Rules, Testing & Quality - -## NEVER Do - -Every item below has caused rejected PRs. Each has a reason — not arbitrary style. - -### Naming -- **NEVER use "Request" for parameter types** — always "Options". The entire SDK uses `{Entity}{Operation}Options`. Using "Request" creates inconsistency and confuses the public API. (`TaskAssignmentOptions`, not `TaskAssignmentRequest`) -- **NEVER use `batch` prefix** for batch methods — use plural names instead. `insertRecordsById`, not `batchInsertRecords`. The SDK convention is singular/plural to distinguish cardinality. - -### Types -- **NEVER use `any` type** — use `unknown` then validate. -- **NEVER use `as unknown as` type casts** — refactor to make types flow naturally. Casts hide real type errors and break when upstream types change. -- **NEVER make all fields required** if the API sometimes omits them — mark optional fields as optional. Over-requiring causes runtime `undefined` access on fields the API didn't return. -- **NEVER leave raw strings/numbers for fixed value sets** — use enums. Raw values lose type safety and autocomplete. If the API returns `1`, `2`, `3` for status, map them to a `Status` enum. -- **NEVER duplicate fields across option types** — extend existing ones. If `CaseInstanceOperationOptions` already has `comment`, extend it instead of re-declaring. When the shape is identical, use `extends` (e.g., `export interface EntityUpdateRecordByIdOptions extends EntityGetRecordByIdOptions {}`) instead of a type alias, as type aliases break TypeDoc documentation generation. -- **NEVER use type aliases for response types** — even when the shape matches an existing one, use an `extends` interface. Type aliases (e.g., `export type EntityUpdateRecordResponse = EntityRecord`) break TypeDoc docs generation by not rendering as standalone types. Use `export interface EntityUpdateRecordResponse extends EntityRecord {}` instead. -- **NEVER write `param || {}` for required parameters** — this hides bugs by silently accepting missing required data at call sites. -- **NEVER put required parameters in an Options object** — required values (IDs, keys, data) are always positional arguments. Options objects are reserved for optional parameters only, always the last argument, always marked `?`. E.g., `getOutput(jobKey: string)` not `getOutput(options: { jobKey: string })`, `close(instanceId, folderKey, options?)` not `close(options: { instanceId, folderKey })`. This is consistent across every service in the SDK. - -### Transforms -- **NEVER add case-only entries to `{Entity}Map`** — field maps are for semantic renames only (`creationTime` → `createdTime`). Case conversion (`CreationTime` → `creationTime`) is handled by `pascalToCamelCaseKeys()`. Mixing them causes double-conversion bugs and makes the map lie about its purpose. -- **NEVER add transform steps without checking the actual API response first** — if the API already returns camelCase, don't add `pascalToCamelCaseKeys()`. If it doesn't return raw enum codes, don't add `applyDataTransforms()`. Each step must be justified by what the API actually sends. -- **NEVER skip transformations** — never return raw API responses; apply applicable pipeline steps. - -### Method binding -- **NEVER bind `getAll()`, `getById()`, `create()`** to response objects — these are service-level entry points, not entity operations. Binding them creates circular nonsense (an entity that retrieves itself). -- **NEVER add `{Entity}Methods` to read-only services** — Assets, Buckets, Queues, Processes, ChoiceSets, Cases, ProcessIncidents have no mutations, so no methods to bind. - -### Internal types -- **NEVER re-export `internal-types.ts` through index.ts** — these are private implementation details (raw API shapes, intermediate parsing types). Re-exporting pollutes the public API and creates breaking-change risk when internal formats change. - -### Endpoints -- **NEVER bypass base service classes** — never directly instantiate HTTP clients; use `this.get()`, `this.post()` from BaseService. -- **NEVER hardcode HTTP method strings in service methods** — use existing constants. Hardcoded strings drift and miss centralized changes. -- **NEVER use inconsistent param names** across endpoints in the same group — if one endpoint uses `instanceId`, all should. Mixing `id`/`instanceId` creates confusion when reading endpoint constants. -- **NEVER use redundant names** in nested groups — under a `CASE` group, use `REOPEN` not `REOPEN_CASE`. The group context already provides the prefix. - -### Error handling -- **NEVER use `ValidationError` for server-side issues** — `ValidationError` is for user input validation only (missing required params, invalid option values). Server-side failures like failed JSON parsing of API responses must use `ServerError`. Using `ValidationError` for parse errors misrepresents the error source and confuses debugging. -- **NEVER add unnecessary type casts on already-typed values** — if `blobAccess.headers` is already `Record`, don't add `arrayDictionaryToRecord()` with `as unknown as` casts. Use a simple spread `{ ...blobAccess.headers }` instead. Redundant casts obscure the actual type and break when the upstream type changes. - -### OperationResponse -- **NEVER use `OperationResponse` for `getAll()`, `getById()`, `create()`** — these return entity data directly. `OperationResponse` is only for state-change operations (cancel, pause, resume) and bulk OData operations with ambiguous success. - -### Pagination -- **NEVER implement pagination manually** — always use `PaginationHelpers.getAll()`. - -### Tests -- **NEVER write test descriptions that don't match the code** — `'should call entity.insert'` is wrong if testing `insertRecord()`. Mismatched descriptions make failures misleading. -- **NEVER hardcode test values** — use existing constants from `tests/utils/constants/`. Hardcoded values drift and hide which test is using which fixture. -- **NEVER leave unused mock methods in mock objects** — dead mocks obscure what the test actually exercises and accumulate as the API evolves. -- **NEVER use `console.log` + `return` in integration test guards** — use `throw new Error()`. Silent skips hide missing test configuration and make CI green when tests aren't actually running. -- **NEVER wrap integration test API calls in try/catch** — let errors propagate naturally. Silent catches mask real failures and make tests pass when they should fail. -- **NEVER create a separate `afterAll` per describe block** if the file already has one — reuse the existing cleanup block by pushing to the shared `createdRecordIds` array. -- **NEVER copy-paste JSDoc comments between endpoint groups** — each constant needs its own comment. A "Asset Service Endpoints" comment on `JOB_ENDPOINTS` is a review rejection. - -### Code hygiene -- **NEVER leave unused code** — unused imports, variables, redundant constructors that only call `super()`. Linter (oxlint) catches these. -- **NEVER add redundant constructors** — if the constructor only calls `super()`, delete it entirely. -- **NEVER commit sensitive files** — `.env`, `credentials.json`, `*.key`, `*.pem`, hardcoded API keys/tokens. - -### Docs -- **NEVER skip `docs/oauth-scopes.md` when adding a method** — every public method needs its scope listed in the same PR. Missing scopes break the OAuth integration guide. -- **NEVER skip `docs/pagination.md` when adding a paginated method** — update the quick reference table with the new method and `jumpToPage` support. Users rely on this table to know which methods support pagination. -- **NEVER skip `mkdocs.yml` when adding a new service** — the new service page won't appear in the docs site navigation without a nav entry. Existing services adding methods don't need this. -- **NEVER skip `@track()` decorator** on public service methods — telemetry gaps are invisible until production debugging, when they're expensive. -- **NEVER use PascalCase in JSDoc examples** — write `id` not `Id`. Examples must match the SDK's post-transform response format or users will write broken code. +# Testing & Quality ## Testing guidelines - Tests use `vitest` with `vi.mock()` and `vi.hoisted()`. Shared mocks in `tests/utils/mocks/`. Use `createMockApiClient()` and `createServiceTestDependencies()` from `tests/utils/setup.ts`. - **Arrange-Act-Assert** pattern. Reset mocks in `afterEach`. - Test both **success and error scenarios** for every public method. -- Test descriptions must match what's being tested. +- Test descriptions must match what's being tested — **NEVER** write `'should call entity.insert'` if testing `insertRecord()`. Mismatched descriptions make failures misleading. - Type request objects in tests — don't leave as untyped objects. -- Use existing test constants from `tests/utils/constants/` instead of hardcoding values. +- Use existing test constants from `tests/utils/constants/` instead of hardcoding values. **NEVER** hardcode test values — hardcoded values drift and hide which test is using which fixture. - **Use domain-specific error constants for entity-specific method tests** (e.g., `getById` → `JOB_TEST_CONSTANTS.ERROR_JOB_NOT_FOUND`, `ASSET_TEST_CONSTANTS.ERROR_ASSET_NOT_FOUND`). Generic `TEST_CONSTANTS.ERROR_MESSAGE` is acceptable for collection methods like `getAll`. Existing pattern: Assets, Queues, Jobs all follow this split. - Verify bound methods exist on response objects when `getById` returns entities with attached methods. - **Validate transform completeness** — for any method with a transform pipeline, verify both: (a) transformed fields have correct values (`result.createdTime`), AND (b) original PascalCase fields are absent (`(result as any).CreationTime` is `undefined`). This catches transform regressions that value-only assertions miss. Existing pattern: Assets (`assets.test.ts:94`), Queues (`queues.test.ts:88`), ChoiceSets (`choicesets.test.ts:243`). - **Every service with bound methods must have a model test file** — `tests/unit/models/{domain}/{entity}.test.ts` testing bound method delegation. This is separate from service tests and verifies that `create{Entity}WithMethods()` correctly binds methods. Existing pattern: Tasks, CaseInstances, ProcessInstances, Entities, Conversations. - **Mock factories must return `Raw{Entity}GetResponse`**, not `{Entity}GetResponse`** — mock factories like `createBasicJob()` produce plain data without bound methods. Methods are attached by the service layer via `create{Entity}WithMethods()`, not by mocks. Using the combined type causes compile errors for missing method properties. - **Shared mock setup belongs in `beforeEach`**, not inline in individual tests** — mock creation like `mockApiClient.getValidToken = vi.fn()` must be in the shared setup block, not duplicated inside each test. +- **NEVER** leave unused mock methods in mock objects — dead mocks obscure what the test actually exercises and accumulate as the API evolves. +- **NEVER** wrap integration test API calls in try/catch — let errors propagate naturally. Silent catches mask real failures and make tests pass when they should fail. +- **NEVER** create a separate `afterAll` per describe block if the file already has one — reuse the existing cleanup block by pushing to the shared `createdRecordIds` array. - **Coverage**: 80% minimum for new code, 100% for critical paths (auth, API calls). - Remove unused mock methods. Extract repeated logic into shared helpers. @@ -88,12 +24,12 @@ Every item below has caused rejected PRs. Each has a reason — not arbitrary st Every new method must also have an integration test in `tests/integration/shared/{domain}/`. These run against a live API and catch issues unit tests miss — wrong endpoints, broken transforms, auth/header problems. -- Use `console.warn()` + skip (not `throw`) for `beforeAll` setup preconditions that are outside the test's control (e.g., no test data available). `throw` is for test body guards where missing config means the test can't run at all. +- Use `console.warn()` + skip (not `throw`) for `beforeAll` setup preconditions that are outside the test's control (e.g., no test data available). `throw` is for test body guards where missing config means the test can't run at all. **NEVER** use `console.log` + `return` for integration test guards — silent skips hide missing test configuration. - Use `getServices()` and `getTestConfig()` from `tests/integration/config/unified-setup.ts` - Use `registerResource()` from `tests/integration/utils/cleanup.ts` for cleanup tracking - Use `generateRandomString()` from `tests/integration/utils/helpers.ts` for unique test data - Tests run in both `v0` and `v1` init modes via `describe.each(modes)` — **only if the service is registered in both modes in `unified-setup.ts`**. New services that only support `v1` init should use `['v1']` only. -- **NEVER write redundant integration tests** — each test must cover a distinct code path, error scenario, or response shape aspect. A test that repeats an existing one with minor value differences (e.g., "should handle job with or without output" duplicating "should return parsed output for a completed job") adds no value and wastes CI time. +- **NEVER** write redundant integration tests — each test must cover a distinct code path, error scenario, or response shape aspect. - **Include a transform validation test** for new methods with a transform pipeline. This test should verify: (a) transformed camelCase fields exist and have values (`job.createdTime`, `job.processName`), AND (b) original PascalCase API fields are absent (`(job as any).CreationTime` is `undefined`, `(job as any).ReleaseName` is `undefined`). This is a separate test from the basic "should retrieve by ID" test — it validates the SDK transform layer against the live API. Note: existing integration tests don't yet follow this pattern, but unit tests do (Assets, Queues, ChoiceSets). Extending it to integration tests catches mismatches between the Swagger spec assumptions and the live API response. ## Documentation @@ -103,18 +39,18 @@ JSDoc comments in `src/models/{domain}/*.models.ts` are the **source of truth fo - `{Entity}ServiceModel` interfaces become the main API reference pages. **Only JSDoc on `{Entity}ServiceModel` is rendered in docs — JSDoc on `{Entity}Methods` does NOT appear.** Place all public-facing documentation on the ServiceModel interface. - Use `@example` with fenced TypeScript blocks, `@param`, `@returns`, `{@link TypeName}`. - Tag internal code with `@internal` or `@ignore`. -- When adding methods, update `docs/oauth-scopes.md` with required OAuth scopes. +- When adding methods, update `docs/oauth-scopes.md` with required OAuth scopes. **NEVER** skip this — missing scopes break the OAuth integration guide. - Run `npm run docs:api` to regenerate. **JSDoc quality rules:** - Link response types with `{@link TypeName}` in every method's JSDoc `@returns`. - Show how to get prerequisite IDs (e.g., "First, get entities with `entities.getAll()`"). - Use `` placeholder convention for IDs in examples. -- Use camelCase in examples, matching SDK response format. +- Use camelCase in examples, matching SDK response format. **NEVER** use PascalCase in JSDoc examples — users will write broken code. - Keep JSDoc in sync with method names. - **When a method supports `expand`**, show multiple expandable entities in the `@example` (e.g., `expand: 'Robot,Machine,Release'`) so users see the comma-separated pattern. - **Add a one-line description of what the response includes** beyond the method signature (e.g., "Returns the full job details including state, timing, and input/output arguments. Use `expand` to include related entities like Robot, Machine, or Release"). -- **NEVER reference unrelated parameters in JSDoc examples** — keep examples focused on the method being documented. If `getOutput()` doesn't accept `folderId`, don't show `folderId` in its example. Unrelated params confuse users about what the method actually accepts. +- **NEVER** reference unrelated parameters in JSDoc examples — keep examples focused on the method being documented. If `getOutput()` doesn't accept `folderId`, don't show `folderId` in its example. - **Show bare minimum call first** in the first `@example`, then a second example with filtering/options. Never use `$` prefix on OData params in examples (`expand` not `$expand`). - **Add JSDoc to non-obvious enum values** — if an enum has values whose meaning isn't clear from the name alone, add a brief comment to each value.