docs: update Claude docs from PR review analysis (#360)

* docs: update claude docs from PR review analysis (2026-04-01 to 2026-04-08)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* docs: update claude docs from PR review analysis (2026-04-06 to 2026-04-13)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* docs: strip source PR refs and reviewer mentions from doc bodies

Doc files should be self-contained guidance, not a changelog. Removes
(Source: PR #N) and @Reviewer references from the new rules/conventions
introduced in this PR. Provenance stays in the PR description.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* docs: remove internal naming rule from rules.md

Co-authored-by: Shreyash <ninja-shreyash@users.noreply.github.com>

* docs: consolidate rules.md anti-patterns into conventions.md

Move all coding anti-patterns (NEVER rules) from rules.md into
conventions.md as inline annotations next to their related conventions.
rules.md now only contains testing, documentation, and quality guidelines.

- Add inline NEVERs to conventions.md only where they add non-obvious
  value (specific traps, unique why) — not mere inversions
- Remove entire NEVER Do section from rules.md, rename to Testing & Quality
- Fold unique test/docs NEVERs into their respective sections in rules.md
- Update enhance-claude-docs skill to route anti-patterns to conventions.md
- Move output/attachment indicator to Service model section per review feedback
- Incorporate PR #360 new insights in correct locations

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-authored-by: shreyash0502 <shreyash9129@gmail.com>
Co-authored-by: Shreyash <ninja-shreyash@users.noreply.github.com>

Author: 4 people

.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.
 

agent_docs/conventions.md

@@ -7,17 +7,21 @@
 - **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
 
 - Services follow the pattern: extend `BaseService`, call `super(uiPath)`, use `this.get()` / `this.post()` etc.
 - 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<TData>` (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<string, string>`, use a simple spread `{ ...blobAccess.headers }` instead of `arrayDictionaryToRecord()` with `as unknown as` casts.
 
 ## BaseService
 
@@ -212,3 +227,9 @@ interface OperationResponse<TData> { 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.