Skip to content

Commit 6d8d845

Browse files
dougborgclaude
andauthored
chore(harness): codify full-regen policy + deferral protocol (#220)
The 2026-05-26 audit surfaced multiple bugs (null supplierCode, null purchaseOrderLineItems/supplier, bare-except parser patterns) that routine full client regenerations would have caught. Root cause: agent briefs in PRs #202 and #216 instructed surgical regens (manually running openapi-python-client on a patched cached spec and copying 1-2 files) to keep diffs small — a pattern that bypasses spec download and lets generated/ drift against the modern generator. Three harness surfaces now codify the policy: - CLAUDE.md "Client regeneration policy" section: default is full pipeline; surgical regens listed under Common Pitfalls. - .claude/skills/regenerate-client/SKILL.md CRITICAL section: always full pipeline; deferral protocol mandates tech-debt issue + regen PR as next work + cross-reference from deferring PR. - .claude/skills/add-nullable-field/SKILL.md step 5: removed the "stash + re-run on clean baseline + reapply" guidance that was encoding the suppress-drift anti-pattern; replaced with absorb-drift default + pointer to the deferral protocol. Deferral is OK when truly necessary but never silent — tracking issue + prioritized follow-up + PR cross-reference are mandatory. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent fecafbd commit 6d8d845

3 files changed

Lines changed: 167 additions & 67 deletions

File tree

.claude/skills/add-nullable-field/SKILL.md

Lines changed: 43 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -13,17 +13,22 @@ Patch `NULLABLE_FIELDS` in the regeneration script, regenerate, validate, and do
1313

1414
## PURPOSE
1515

16-
Make a generated model field nullable so the client stops crashing on null API responses — without adding `# type: ignore`.
16+
Make a generated model field nullable so the client stops crashing on null API responses
17+
— without adding `# type: ignore`.
1718

1819
## CRITICAL
1920

20-
- **Never use `# type: ignore` to silence the error** — that's a forbidden shortcut per CLAUDE.md. Add the field to `NULLABLE_FIELDS` and regenerate.
21-
- **Never edit `stocktrim_public_api_client/generated/` directly** — changes are lost on regeneration.
22-
- **The fix must be reproducible from the spec** — anyone running `uv run poe regenerate-client` must get the same result.
21+
- **Never use `# type: ignore` to silence the error** — that's a forbidden shortcut per
22+
CLAUDE.md. Add the field to `NULLABLE_FIELDS` and regenerate.
23+
- **Never edit `stocktrim_public_api_client/generated/` directly** — changes are lost on
24+
regeneration.
25+
- **The fix must be reproducible from the spec** — anyone running
26+
`uv run poe regenerate-client` must get the same result.
2327

2428
## ASSUMES
2529

26-
- You have an actual error message identifying the schema and field (e.g., `PurchaseOrderResponseDto.orderDate` returns null but spec marks it required).
30+
- You have an actual error message identifying the schema and field (e.g.,
31+
`PurchaseOrderResponseDto.orderDate` returns null but spec marks it required).
2732
- `scripts/regenerate_client.py` exists and contains a `NULLABLE_FIELDS` dict.
2833

2934
## STANDARD PATH
@@ -53,7 +58,8 @@ NULLABLE_FIELDS = {
5358
}
5459
```
5560

56-
Use the same comment style as existing entries. Mark with `⚠️ CRITICAL` if null actually crashes (not just wrong types).
61+
Use the same comment style as existing entries. Mark with `⚠️ CRITICAL` if null actually
62+
crashes (not just wrong types).
5763

5864
### 3. Regenerate
5965

@@ -67,30 +73,53 @@ uv run poe regenerate-client
6773
uv run poe check
6874
```
6975

70-
ALL must pass. If a different field is now broken, repeat from step 1 — don't bail with `# type: ignore`.
76+
ALL must pass. If a different field is now broken, repeat from step 1 — don't bail with
77+
`# type: ignore`.
7178

72-
### 5. Confirm the diff is bounded
79+
### 5. Inspect the diff and absorb any upstream drift
7380

7481
```bash
7582
git diff --stat
7683
```
7784

78-
You should see:
85+
You'll see at minimum:
7986

8087
- `scripts/regenerate_client.py` (1 small change)
81-
- `stocktrim_public_api_client/generated/models/<schema>.py` (the field becomes `Optional`)
88+
- `stocktrim_public_api_client/generated/models/<schema>.py` (the field becomes
89+
`Optional`)
8290
- Possibly `client_types.py` if reexports change
8391

84-
If the diff is much bigger, regeneration picked up an unrelated upstream change. Stash, run regeneration on a clean baseline, then re-apply your `NULLABLE_FIELDS` change.
92+
**If the diff is bigger than that, it's because the live StockTrim spec drifted since
93+
the last regen — this is expected and good.** The regen pulls the latest spec; **default
94+
is to absorb that drift in the same PR as additional commits.** Adapt downstream code
95+
(helpers, services, tests, MCP tools) to match any new field shapes, renames, or
96+
removals. Document the broader changes in the commit body so reviewers see what changed
97+
in the API surface.
98+
99+
If the drift introduces sprawling adaptations that would meaningfully delay your
100+
nullable-field fix and absolutely cannot ride along, follow the **deferral protocol** in
101+
`.claude/skills/regenerate-client/SKILL.md` (file `tech-debt` tracking issue + open the
102+
regen PR next + reference from this PR's description). Deferring without those three
103+
steps is the anti-pattern that caused the 2026-05 drift bugs.
104+
105+
**DO NOT** stash, reset, or otherwise try to suppress the drift to keep the diff small.
106+
Surgical regens that only touch 1–2 files are an **anti-pattern**; they bypass the live
107+
spec download and leave the rest of `generated/` frozen against an old generator.
85108

86109
### 6. Document evidence (optional)
87110

88-
If `docs/contributing/api-feedback.md` exists, append a row noting the schema + field + observed behavior. This is the canonical record for "spec says required, API returns null."
111+
If `docs/contributing/api-feedback.md` exists, append a row noting the schema + field +
112+
observed behavior. This is the canonical record for "spec says required, API returns
113+
null."
89114

90115
## EDGE CASES
91116

92-
- [Field is in a deeply nested array item]`NULLABLE_FIELDS` works at the top level of a schema. For nested types, check whether the nested schema is itself a top-level component (it usually is) and add it there.
93-
- [Field is required in some endpoints but not others]`NULLABLE_FIELDS` marks the schema field nullable everywhere it's used. Acceptable trade-off; document in the comment.
117+
- [Field is in a deeply nested array item]`NULLABLE_FIELDS` works at the top level of
118+
a schema. For nested types, check whether the nested schema is itself a top-level
119+
component (it usually is) and add it there.
120+
- [Field is required in some endpoints but not others]`NULLABLE_FIELDS` marks the
121+
schema field nullable everywhere it's used. Acceptable trade-off; document in the
122+
comment.
94123

95124
## RELATED
96125

.claude/skills/regenerate-client/SKILL.md

Lines changed: 50 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -10,23 +10,44 @@ allowed-tools: Bash(uv run poe regenerate-client), Bash(uv run poe check), Bash(
1010

1111
# /regenerate-client — Regenerate the StockTrim API Client
1212

13-
Run the OpenAPI regeneration pipeline, validate, and commit the result on a feature branch.
13+
Run the OpenAPI regeneration pipeline, validate, and commit the result on a feature
14+
branch.
1415

1516
## PURPOSE
1617

17-
Replace the generated client with a fresh build from the live spec, with all post-processing applied.
18+
Replace the generated client with a fresh build from the live spec, with all
19+
post-processing applied.
1820

1921
## CRITICAL
2022

21-
- **Never hand-edit `stocktrim_public_api_client/generated/` or `client_types.py`** — both are wholly replaced by this skill. Edits are silently lost.
22-
- **All type/null fixes go in `scripts/regenerate_client.py`** — never add `# type: ignore` to generated code; add the field to `NULLABLE_FIELDS` instead.
23+
- **ALWAYS use the full pipeline (`uv run poe regenerate-client`).** Surgical regens —
24+
manually running `openapi-python-client generate` against the cached spec and copying
25+
1–2 files into `generated/` — are an **ANTI-PATTERN**. They bypass the live spec
26+
download, leave the rest of `generated/` frozen against the older generator, and
27+
accumulate drift bugs (stale field types, bare-except patterns, missing nullable
28+
markers). If you need to regenerate ANYTHING, regenerate EVERYTHING.
29+
- **Default: absorb upstream drift in the same PR as additional commits.** Keep the
30+
immediate-fix commit focused, then add follow-up commits adapting
31+
helpers/services/tests for whatever the regen brought in. Do not stash or reset to
32+
keep the diff small.
33+
- **Deferral protocol (only when you absolutely cannot ship the regen in the same PR):**
34+
ALL of the following are mandatory:
35+
1. File a `tech-debt` tracking issue noting the regen is owed and why it was deferred.
36+
1. Open the regen PR as the very next piece of work — no unrelated tasks first.
37+
1. Reference the tracking issue from the deferring PR's description.
38+
- **Never hand-edit `stocktrim_public_api_client/generated/` or `client_types.py`**
39+
both are wholly replaced by this skill. Edits are silently lost.
40+
- **All type/null fixes go in `scripts/regenerate_client.py`** — never add
41+
`# type: ignore` to generated code; add the field to `NULLABLE_FIELDS` instead.
2342
- **Must be on a feature branch** — never regenerate directly on `main`.
24-
- **CLAUDE.md zero-tolerance applies**`uv run poe check` must be green before committing.
43+
- **CLAUDE.md zero-tolerance applies**`uv run poe check` must be green before
44+
committing.
2545

2646
## ASSUMES
2747

2848
- You're in a git repository on a feature branch (or willing to create one).
29-
- The live StockTrim spec at `https://api.stocktrim.com/swagger/v1/swagger.yaml` is reachable.
49+
- The live StockTrim spec at `https://api.stocktrim.com/swagger/v1/swagger.yaml` is
50+
reachable.
3051
- `uv` and project deps are installed (`uv sync`).
3152

3253
## STANDARD PATH
@@ -49,14 +70,14 @@ uv run poe regenerate-client
4970
This invokes `scripts/regenerate_client.py` which:
5071

5172
1. Downloads the spec
52-
2. Patches auth (header params → securitySchemes)
53-
3. Validates with `openapi-spec-validator` and Redocly
54-
4. Generates the client via `openapi-python-client`
55-
5. Renames `types.py``client_types.py` and rewrites imports
56-
6. Modernizes `Union[X, Y]``X | Y`
57-
7. Fixes RST docstrings
58-
8. Applies `NULLABLE_FIELDS` overrides
59-
9. Runs `ruff --fix`
73+
1. Patches auth (header params → securitySchemes)
74+
1. Validates with `openapi-spec-validator` and Redocly
75+
1. Generates the client via `openapi-python-client`
76+
1. Renames `types.py``client_types.py` and rewrites imports
77+
1. Modernizes `Union[X, Y]``X | Y`
78+
1. Fixes RST docstrings
79+
1. Applies `NULLABLE_FIELDS` overrides
80+
1. Runs `ruff --fix`
6081

6182
### 3. Inspect the diff
6283

@@ -75,9 +96,12 @@ uv run poe check
7596

7697
ALL must pass. If failures appear:
7798

78-
- **Type error on null field?** → Add to `NULLABLE_FIELDS` in `scripts/regenerate_client.py`, re-run from Step 2. Never add `# type: ignore`.
79-
- **Helper method broken by renamed model?** → Update the helper. Helpers wrap generated/, so they need to track renames.
80-
- **MCP tool broken?** → Update the corresponding service in `stocktrim_mcp_server/src/.../services/`.
99+
- **Type error on null field?** → Add to `NULLABLE_FIELDS` in
100+
`scripts/regenerate_client.py`, re-run from Step 2. Never add `# type: ignore`.
101+
- **Helper method broken by renamed model?** → Update the helper. Helpers wrap
102+
generated/, so they need to track renames.
103+
- **MCP tool broken?** → Update the corresponding service in
104+
`stocktrim_mcp_server/src/.../services/`.
81105

82106
### 5. Commit on feature branch
83107

@@ -95,17 +119,22 @@ EOF
95119
)"
96120
```
97121

98-
Use `feat(client):` scope for client release; add `chore(mcp):` companion commit if MCP services were updated.
122+
Use `feat(client):` scope for client release; add `chore(mcp):` companion commit if MCP
123+
services were updated.
99124

100125
## EDGE CASES
101126

102-
- [Spec download fails] — Check `SPEC_URL` in `scripts/regenerate_client.py`. Network issue? Retry. Permanent? Open an issue.
127+
- [Spec download fails] — Check `SPEC_URL` in `scripts/regenerate_client.py`. Network
128+
issue? Retry. Permanent? Open an issue.
103129
- [Generation breaks for an unrelated schema] — Read DETAIL: Quarantining a Schema
104-
- [Helper or test fails after regen] — Update the helper/test. Generated code is the source of truth; downstream code adapts.
130+
- [Helper or test fails after regen] — Update the helper/test. Generated code is the
131+
source of truth; downstream code adapts.
105132

106133
## DETAIL: Quarantining a Schema
107134

108-
If a schema generates uncompilable code (rare), patch it in `scripts/regenerate_client.py` before the generation step rather than editing the generated output.
135+
If a schema generates uncompilable code (rare), patch it in
136+
`scripts/regenerate_client.py` before the generation step rather than editing the
137+
generated output.
109138

110139
## RELATED
111140

CLAUDE.md

Lines changed: 74 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,8 @@ uv run poe test-mcp # Run MCP server tests (stocktrim_mcp_server/te
112112
uv run poe test-coverage # Run tests with coverage reports (HTML, terminal, XML)
113113
```
114114

115-
`uv run poe check` runs both `test` and `test-mcp`, so the MCP server suite cannot regress silently when only the client lib is touched.
115+
`uv run poe check` runs both `test` and `test-mcp`, so the MCP server suite cannot
116+
regress silently when only the client lib is touched.
116117

117118
### Documentation
118119

@@ -180,6 +181,8 @@ this format for consistency.
180181
- Call real APIs in tests
181182
- Leave debug code or commented sections
182183
- Ignore pre-existing test failures or linting errors
184+
- **Surgical regens** — manually running `openapi-python-client generate` and copying
185+
1–2 files into `generated/`
183186

184187
**Do this instead:**
185188

@@ -189,51 +192,90 @@ this format for consistency.
189192
- Mock all external dependencies
190193
- Remove all debug code before PR
191194
- Fix ALL tests and linting issues, regardless of when they were introduced
195+
- **Always run the full regeneration pipeline (`uv run poe regenerate-client`)** when
196+
touching `generated/`. Absorb the upstream spec drift in the same PR.
197+
198+
## Client regeneration policy
199+
200+
The StockTrim OpenAPI spec evolves continuously. **Always pull the latest spec and adapt
201+
downstream code as necessary.**
202+
203+
- **Default path**: `uv run poe regenerate-client` (downloads latest spec → patches →
204+
regenerates everything → ruff post-processing).
205+
206+
- **Surgical regens** (manually running `openapi-python-client generate` on a cached
207+
spec and copying individual files) are an **ANTI-PATTERN**. They bypass the spec
208+
download, leave the rest of `generated/` stale against the modern generator, and
209+
accumulate invisible drift bugs (stale field types, bare-except patterns, missing
210+
nullable markers, etc.).
211+
212+
- **Drift compounds.** Six months of deferred regens caused multiple production bugs in
213+
2026-05 (null `supplierCode`, null `purchaseOrderLineItems`/`supplier`, bare-except
214+
parser patterns). All would have been caught by routine full regens.
215+
216+
- **Embrace the diff.** When a full regen pulls in changes beyond your immediate fix,
217+
the **default is to ship the broader adaptations in the same PR** as additional
218+
commits — keep the immediate-fix commit focused, then add follow-up commits adapting
219+
helpers/services/tests to absorb the drift.
220+
221+
- **Deferral protocol.** If absorbing the full regen genuinely cannot ride in the same
222+
PR (e.g. the immediate fix is urgent and the drift introduces sprawling adaptations
223+
that would block it), then ALL THREE of the following are mandatory:
224+
225+
1. **File a tracking issue** (labelled `tech-debt`) noting that a regen is owed, with
226+
the specific reason it was deferred.
227+
1. **Open the regen PR as the very next piece of work** — do not start unrelated tasks
228+
first. The drift doesn't sit on the shelf.
229+
1. **Reference the tracking issue from the immediate-fix PR description** so reviewers
230+
and future-you see the debt explicitly.
231+
232+
Deferring without these three steps is the same anti-pattern that caused the 2026-05
233+
bugs. Don't.
234+
235+
See `.claude/skills/regenerate-client/SKILL.md` for the full procedure.
192236

193237
## Agent Harness
194238

195-
This repo uses the [harness-kit](https://github.com/dougborg/harness-kit) plugin
196-
for Claude Code, with project-specific extensions in `.claude/`. Provenance is
197-
tracked in `.harness-lock.json`. Run `/harness-kit:harness` to audit, update, or
198-
retro the harness.
239+
This repo uses the [harness-kit](https://github.com/dougborg/harness-kit) plugin for
240+
Claude Code, with project-specific extensions in `.claude/`. Provenance is tracked in
241+
`.harness-lock.json`. Run `/harness-kit:harness` to audit, update, or retro the harness.
199242

200243
### Skills
201244

202-
| Skill | Purpose |
203-
| --- | --- |
204-
| `/commit` | Quality-gated conventional commits (upstream) |
205-
| `/feature-spec` | Spec before multi-file implementations (upstream) |
206-
| `/open-pr` | Push, create PR, wait for CI, address first round (upstream) |
207-
| `/code-reviewer` | Six-dimension review with project-specific BLOCKING rules |
208-
| `/skill-writer` | Author new skills with progressive disclosure (upstream) |
209-
| `/standup` | Git/GitHub activity recap (upstream) |
210-
| `/regenerate-client` | Run the OpenAPI regeneration pipeline + commit |
245+
| Skill | Purpose |
246+
| --------------------- | ------------------------------------------------------------ |
247+
| `/commit` | Quality-gated conventional commits (upstream) |
248+
| `/feature-spec` | Spec before multi-file implementations (upstream) |
249+
| `/open-pr` | Push, create PR, wait for CI, address first round (upstream) |
250+
| `/code-reviewer` | Six-dimension review with project-specific BLOCKING rules |
251+
| `/skill-writer` | Author new skills with progressive disclosure (upstream) |
252+
| `/standup` | Git/GitHub activity recap (upstream) |
253+
| `/regenerate-client` | Run the OpenAPI regeneration pipeline + commit |
211254
| `/add-nullable-field` | Patch `NULLABLE_FIELDS` and regenerate (no `# type: ignore`) |
212-
| `/add-helper-method` | Add a typed helper that wraps a generated call |
213-
| `/add-mcp-tool` | Add an MCP tool + service + test |
214-
| `/quality-gate` | CLAUDE.md zero-tolerance pre-done checklist |
255+
| `/add-helper-method` | Add a typed helper that wraps a generated call |
256+
| `/add-mcp-tool` | Add an MCP tool + service + test |
257+
| `/quality-gate` | CLAUDE.md zero-tolerance pre-done checklist |
215258

216259
### Agents
217260

218-
| Agent | Purpose | Model |
219-
| --- | --- | --- |
220-
| `code-reviewer` | Pre-PR review (6 dimensions + StockTrim-specific blockers) | sonnet |
221-
| `verifier` | Final-gate check: validation, branch, no shortcuts | haiku |
222-
| `test-writer` | Pytest tests with httpx transport mocks | sonnet |
223-
| `domain-advisor` | Read-only Q&A on StockTrim API quirks, retry policy, auth | sonnet |
224-
| `project-manager` | GitHub issues, PRs, releases, dependabot triage | sonnet |
261+
| Agent | Purpose | Model |
262+
| ----------------- | ---------------------------------------------------------- | ------ |
263+
| `code-reviewer` | Pre-PR review (6 dimensions + StockTrim-specific blockers) | sonnet |
264+
| `verifier` | Final-gate check: validation, branch, no shortcuts | haiku |
265+
| `test-writer` | Pytest tests with httpx transport mocks | sonnet |
266+
| `domain-advisor` | Read-only Q&A on StockTrim API quirks, retry policy, auth | sonnet |
267+
| `project-manager` | GitHub issues, PRs, releases, dependabot triage | sonnet |
225268

226269
### Automation Philosophy
227270

228271
`.claude/settings.json` configures PostToolUse hooks in three tiers:
229272

230273
1. **Formatters** (silent, zero-token) — `ruff check --fix` and `ruff format` run
231274
automatically on every Python edit so style issues never reach Claude.
232-
2. **Validators** (bounded, gated) — `ty check` and per-file `pytest` run on the
233-
touched file only; output is capped at ≤25 lines so noise never drowns signal.
234-
3. **Guidance** (≤20 lines) — generated/ edits, regenerate_client.py changes,
235-
and OpenAPI spec touches print a one-line nudge pointing to the right skill.
236-
237-
A Stop hook nudges toward `/harness-kit:harness retro` after sessions touching
238-
more than three files. Never silence these by removing them — fix the cause they
239-
surface.
275+
1. **Validators** (bounded, gated) — `ty check` and per-file `pytest` run on the touched
276+
file only; output is capped at ≤25 lines so noise never drowns signal.
277+
1. **Guidance** (≤20 lines) — generated/ edits, regenerate_client.py changes, and
278+
OpenAPI spec touches print a one-line nudge pointing to the right skill.
279+
280+
A Stop hook nudges toward `/harness-kit:harness retro` after sessions touching more than
281+
three files. Never silence these by removing them — fix the cause they surface.

0 commit comments

Comments
 (0)