Skip to content

Commit d071588

Browse files
Strengthen agent rules from APIError porting learnings (#14)
* Strengthen agent rules from APIError porting learnings * update * Update .agent/rules/porting.mdc Co-authored-by: mihaimitrea-db <mihai.mitrea@databricks.com> --------- Co-authored-by: mihaimitrea-db <mihai.mitrea@databricks.com>
1 parent 9a26ef9 commit d071588

4 files changed

Lines changed: 100 additions & 5 deletions

File tree

.agent/rules/porting.mdc

Lines changed: 41 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -71,12 +71,33 @@ const nullishString = z.string().nullish().transform(v => v ?? '');
7171
const goString = z.string().nullish().transform(v => v ?? '');
7272
```
7373

74+
## Critical Thinking
75+
76+
**Do not blindly copy.** Before porting each function, type, or test, ask:
77+
78+
1. **Does TypeScript need it?** Some Go code exists only because Go requires
79+
explicit opt-in for features that TypeScript provides natively. For example,
80+
Go requires an explicit `Unwrap() error` method for error chain traversal;
81+
TypeScript gets this for free via `Error.cause`. Do not port such code.
82+
2. **Does the test validate our code or the language?** If a Go test verifies
83+
behaviour that is built into the TypeScript runtime (e.g. that `Error.cause`
84+
preserves a reference), skip it — it tests the language, not our
85+
implementation. Explain the skip to the user.
86+
87+
When in doubt, ask the user before porting.
88+
89+
## Factory Functions vs Constructors
90+
91+
When a Go function can return `nil` (e.g. `FromHTTPError` returns `nil` for 2xx
92+
status codes), port it as a **factory function** that returns `T | undefined`.
93+
Constructors in TypeScript always return an instance and cannot express "no
94+
result." Use a standalone function or a static method on the class.
95+
7496
## Tests
7597

76-
Port **all** test cases from the Go SDK. Do not selectively skip tests. When a
77-
Go test case cannot be directly ported (e.g. because of a language difference
78-
like `json.RawMessage` vs already-parsed JSON), explain the omission to the
79-
user and get confirmation before skipping.
98+
Port **all** test cases from the Go SDK unless they fall under the "Critical
99+
Thinking" exceptions above. Do not selectively skip tests without explaining
100+
the reason to the user.
80101

81102
Use the same test structure as the Go SDK. Go table-driven tests map to
82103
Vitest's `it.each`:
@@ -101,6 +122,22 @@ const testCases: {name: string; input: number; want: number}[] = [
101122
it.each(testCases)('$name', ({input, want}) => { ... });
102123
```
103124

125+
## No Go References in Code
126+
127+
**Never reference Go in code or comments.** This includes variable names,
128+
function names, inline comments, and JSDoc. The TypeScript codebase must read
129+
as if Go does not exist. Do not write comments like "mirrors Go's
130+
tc.want.httpErr" or "equivalent of Go's errors.As."
131+
132+
```typescript
133+
// Bad — references the Go implementation.
134+
// Input HTTP fields are always preserved (mirrors Go's
135+
// tc.want.httpErr = &httpError{statusCode, header, body}).
136+
137+
// Good — describes what the code does without referencing Go.
138+
// Input HTTP fields are always preserved.
139+
```
140+
104141
## Deviations from Go
105142

106143
When the TypeScript implementation must differ from Go (e.g. different function

.agent/rules/testing.mdc

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,48 @@ expect(token.accessToken).toBe('dapi...');
8787
expect(result).toStrictEqual({host: 'example.com', port: 443});
8888
```
8989

90+
## Never Silently Skip Assertions
91+
92+
Do not use `return` in a test path where a failure should be reported. If a
93+
precondition fails, use `expect.fail()` so the test fails loudly instead of
94+
silently passing with no assertions.
95+
96+
```typescript
97+
// Good — fails loudly if got is unexpectedly undefined.
98+
if (got === undefined) {
99+
expect.fail('expected a result');
100+
}
101+
102+
// Bad — silently skips all remaining assertions.
103+
if (got === undefined) {
104+
return;
105+
}
106+
```
107+
108+
## Test All Outputs
109+
110+
When a function returns an object, verify all its fields — not just the ones
111+
that seem interesting. Do not skip fields because they appear "obvious" or are
112+
"just pass-through." If the function sets a field, the test should check it.
113+
114+
## Use Real Types for Expected Values
115+
116+
When the expected output is a class instance, use that class as the type of
117+
the `want` field in test tables. Do not invent anonymous object types that
118+
duplicate the class shape.
119+
120+
```typescript
121+
// Good — want is a real APIError.
122+
const testCases: {desc: string; want?: APIError}[] = [
123+
{desc: 'not found', want: new APIError({code: Code.NOT_FOUND, ...})},
124+
];
125+
126+
// Bad — anonymous type duplicates APIError's shape.
127+
const testCases: {desc: string; want?: {code: Code; message: string}}[] = [
128+
{desc: 'not found', want: {code: Code.NOT_FOUND, message: '...'}},
129+
];
130+
```
131+
90132
## Independence
91133

92134
Each test must be independent. Do not rely on execution order. Clean up side

.agent/rules/typescript.mdc

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -443,6 +443,12 @@ Avoid private static methods; prefer module-level functions instead. Never use
443443

444444
Always include parentheses in constructor calls: `new Foo()`, not `new Foo`.
445445

446+
### 7.7 API Surface Minimization
447+
448+
Default to the smallest public API surface. Only export from `index.ts` what
449+
consumers need. Internal types (options interfaces, schemas, helper functions)
450+
must not be re-exported.
451+
446452
---
447453

448454
## 8. Control Flow

AGENTS.md

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,4 +99,14 @@ npm run clean
9999
with a period.
100100
6. **Back up claims.** When proposing a design decision or asserting a
101101
convention, provide concrete references (documentation, API links). Do not
102-
state something is "idiomatic" or "standard" without evidence.
102+
state something is "idiomatic" or "standard" without evidence. Use
103+
authoritative primary sources (language specs, official documentation) — not
104+
blog posts, archived repositories, or npm packages.
105+
7. **Stay in scope.** Only change what was asked. Each change should be
106+
reviewable in isolation.
107+
8. **Never silently remove code.** If a change requires deleting existing
108+
code or tests, explain what is being removed and why **before** proceeding.
109+
Get explicit confirmation from the user.
110+
9. **Match existing patterns.** Before writing new code, check existing code
111+
for established patterns. Do not invent new conventions
112+
when the codebase already has one.

0 commit comments

Comments
 (0)