From 2a80407d92c5f0d1af662d468437178db19f2bf9 Mon Sep 17 00:00:00 2001 From: ShocOne <62835948+ShocOne@users.noreply.github.com> Date: Wed, 26 Aug 2026 13:00:16 +0100 Subject: [PATCH 1/3] test: the quirkserver can name a refused field in prose MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An API refuses a create two ways. It can leave the bare field name in a detail beside a generic title, which is deliberately unhealable — a name alone could be about anything, and the adjustment loop must not guess. Or it can spell the field into a sentence, which is the form the loop reads. The quirkserver could only do the first, so no test could exercise a create the loop heals and a later step then replays. NamesRefusedFieldInProse gives it the second, with an exhibit asserting both forms. The recipe also learns the body that worked. Everything downstream replays it — re-creating this entity as another's parent, narrowing a refused maximal, cleaning up at the end — and the body the plan started from is the document's guess, which is what needed healing. No test here demonstrates a changed outcome: each of those paths heals again on its own, so the guess survives being replayed. It is still the wrong thing to carry. Co-Authored-By: Claude Opus 5 (1M context) --- internal/audit/run/adaptive_test.go | 52 ++++++++++++++++++++++ internal/audit/run/steps_create.go | 6 +++ internal/quirkserver/behaviour.go | 9 ++++ internal/quirkserver/exhibit_write_test.go | 26 +++++++++++ internal/quirkserver/handlers.go | 2 +- internal/quirkserver/quirks.go | 10 +++++ 6 files changed, 104 insertions(+), 1 deletion(-) diff --git a/internal/audit/run/adaptive_test.go b/internal/audit/run/adaptive_test.go index 7695b78..e812784 100644 --- a/internal/audit/run/adaptive_test.go +++ b/internal/audit/run/adaptive_test.go @@ -492,3 +492,55 @@ func TestUnit_Adaptive_RedactionHoldsEndToEnd(t *testing.T) { // The run's activity ledger must not carry it either. _ = time.Now } + +// TestUnit_Adaptive_TheReductionKeepsWhatTheCreateNeeded: a field the API +// demanded at createMinimal is not something the maximal reduction may drop. +// +// The reduction protects the body the entity creates by. That body is the one +// the run got accepted, not the one the plan started from — the plan's is the +// document's guess, and the guess is what needed healing. Protecting the guess +// leaves the healed field droppable, and a reduction that drops it can never +// reach an accepted create at all. +func TestUnit_Adaptive_TheReductionKeepsWhatTheCreateNeeded(t *testing.T) { + t.Parallel() + s := quirkserver.New(t, quirkserver.Quirks{ + // Demanded by the API, absent from the document, and named in a + // sentence the loop can act on. + RequiredButUndeclared: []string{"serial"}, + NamesRefusedFieldInProse: true, + // One optional field the maximal carries and the API will not take, + // so the maximal is refused and has to be reduced. + RejectsDocumentedValue: map[string]string{"colour": "bad-colour"}, + }) + + p := &plan.Plan{ + Entities: []plan.EntityPlan{{ + Entity: "thing", Role: "resource", Budget: plan.Budget{Requests: 60}, + Steps: []plan.Step{ + {Kind: plan.StepCreateMinimal, Method: "POST", Path: "/things", + Body: map[string]any{"name": "tfpfgen--thing-name"}}, + {Kind: plan.StepCreateMaximal, Method: "POST", Path: "/things", + Body: map[string]any{"name": "tfpfgen--thing-name", "colour": "bad-colour"}}, + }, + }}, + Budget: plan.RunBudget{Requests: 300, Objects: 10, Duration: "1m"}, + } + + _, sum := mustRun(t, testOptions(t, s, p, testEnv(), nil)) + + var recorded *observe.AcceptedBody + for _, b := range sum.Bodies { + if b.Entity == "thing" { + recorded = b.Maximal + } + } + if recorded == nil { + t.Fatal("no maximal create was ever accepted: the reduction dropped what the create needed") + } + if _, kept := recorded.Request["serial"]; !kept { + t.Errorf("the reduction dropped the field the API demanded: %#v", recorded.Request) + } + if _, dropped := recorded.Request["colour"]; dropped { + t.Errorf("the reduction kept the field the API refused: %#v", recorded.Request) + } +} diff --git a/internal/audit/run/steps_create.go b/internal/audit/run/steps_create.go index 63bd982..98ca180 100644 --- a/internal/audit/run/steps_create.go +++ b/internal/audit/run/steps_create.go @@ -39,6 +39,12 @@ func (r *runner) runCreateMinimal(ctx context.Context, ent *entityState, step *p if err != nil { return err } + // The recipe learns the body that worked. Everything downstream + // replays it — re-creating this entity as another's parent, narrowing + // a refused maximal, cleaning up at the end — and the body the plan + // started from is the document's guess, which is what needed healing + // in the first place. + ent.recipe.minimalBody = cloneAnyMap(rr.body) r.registry[ent.plan.Entity] = rr.obj ent.createdAt = time.Now() ent.ev.sent = sent diff --git a/internal/quirkserver/behaviour.go b/internal/quirkserver/behaviour.go index 650098d..fd1de45 100644 --- a/internal/quirkserver/behaviour.go +++ b/internal/quirkserver/behaviour.go @@ -66,6 +66,15 @@ func (s *Server) normalise(field string, v any) any { return v } +// refusedFieldDetail renders the missing field as the quirk asks: a bare name, +// or a sentence an adjustment loop can read. +func (s *Server) refusedFieldDetail(field string) string { + if s.quirks.NamesRefusedFieldInProse { + return "field " + field + " is required" + } + return field +} + func (s *Server) missingRequired(body map[string]any) string { for _, field := range s.quirks.RequiredButUndeclared { if _, ok := body[field]; !ok { diff --git a/internal/quirkserver/exhibit_write_test.go b/internal/quirkserver/exhibit_write_test.go index 4be50b8..2264832 100644 --- a/internal/quirkserver/exhibit_write_test.go +++ b/internal/quirkserver/exhibit_write_test.go @@ -199,6 +199,32 @@ var writeExhibits = map[string]func(*testing.T){ } }, + "NamesRefusedFieldInProse": func(t *testing.T) { + t.Parallel() + + // The same missing field, said two ways. Only the sentence is one an + // adjustment loop can act on; the bare name beside a generic title + // could be about anything, which is why it stays unhealable. + bare := New(t, Quirks{RequiredButUndeclared: []string{"key"}}) + _, body := post(t, bare.CollectionURL(), map[string]any{"value": "v"}) + if detail, _ := body["detail"].(string); detail != "key" { + t.Errorf("without the quirk the detail should be the bare name, got %v", body) + } + + prose := New(t, Quirks{RequiredButUndeclared: []string{"key"}, NamesRefusedFieldInProse: true}) + status, body := post(t, prose.CollectionURL(), map[string]any{"value": "v"}) + if status != http.StatusBadRequest { + t.Fatalf("status = %d, want 400", status) + } + if detail, _ := body["detail"].(string); detail != "field key is required" { + t.Errorf("the refusal should spell the field into a sentence, got %v", body) + } + + if status, _ := post(t, prose.CollectionURL(), map[string]any{"key": "k"}); status != http.StatusCreated { + t.Errorf("supplying it should succeed, got %d", status) + } + }, + "ConditionallyRequired": func(t *testing.T) { t.Parallel() diff --git a/internal/quirkserver/handlers.go b/internal/quirkserver/handlers.go index 0082e09..09885e9 100644 --- a/internal/quirkserver/handlers.go +++ b/internal/quirkserver/handlers.go @@ -48,7 +48,7 @@ func (s *Server) create(w http.ResponseWriter, r *http.Request) { // Naming the field is what lets the auditor write the requirement down // as observed rather than guessed: an error that does not name it // could have been about anything. - s.fail(w, http.StatusBadRequest, "missing required field", missing) + s.fail(w, http.StatusBadRequest, "missing required field", s.refusedFieldDetail(missing)) return } diff --git a/internal/quirkserver/quirks.go b/internal/quirkserver/quirks.go index 48b31ae..1b91ba4 100644 --- a/internal/quirkserver/quirks.go +++ b/internal/quirkserver/quirks.go @@ -117,6 +117,16 @@ type Quirks struct { // EventuallyConsistentReads makes the first N reads of a new object 404. EventuallyConsistentReads int + // NamesRefusedFieldInProse makes a refusal spell the missing field into a + // sentence — "field serial is required" — rather than leaving the bare + // name in the detail beside a generic title. + // + // Both are real. The bare form is deliberately unhealable, because a name + // alone beside a generic title could be about anything; this is the form + // an adjustment loop can act on, and an API that writes it is one whose + // refusals teach the auditor what the create needs. + NamesRefusedFieldInProse bool + // ErrorEnvelope selects which shape errors take. Defaults to problem+json. ErrorEnvelope Envelope From 32a7f8c60b598673592a2f16314064d6f3fb79bb Mon Sep 17 00:00:00 2001 From: ShocOne <62835948+ShocOne@users.noreply.github.com> Date: Wed, 26 Aug 2026 13:22:13 +0100 Subject: [PATCH 2/3] fix: the maximal create runs once the minimal object is gone MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit No entity ever recorded a maximal body. The strategy program created the widest valid body while the object the minimal create had just made was still live, and both bodies are synthesised from the same document — so an API that keys an object on any field they share answered the second create with a conflict: attempt 0 400 invalid icon -> drop icon attempt 1 409 conflict attempt 2 409 conflict -> and so on, to the allowance The reduction was working the whole time. It dropped what the API named and then had nothing left to learn from, because a conflict says nothing about how wide a valid body is. The maximal create makes a second object and deletes it again, so it belongs after the delete rather than before it. Recorded maximal bodies follow, and with them a maximal configuration that is wider than the minimal one and excludes what the API accepts without ever returning: color = "#FF0000" description = "To tag assets in San Francisco" # match_type skipped: the API accepted this property and did not return it Co-Authored-By: Claude Opus 5 (1M context) --- internal/audit/strategy/program.go | 21 +++++++++------- internal/audit/strategy/strategy_test.go | 32 ++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 9 deletions(-) diff --git a/internal/audit/strategy/program.go b/internal/audit/strategy/program.go index d53164e..453b566 100644 --- a/internal/audit/strategy/program.go +++ b/internal/audit/strategy/program.go @@ -135,10 +135,6 @@ func buildProgram(createBody *specmodel.Schema, gates []Gate, variants []Variant gatedStep(stepReadConsecutive, v), ) } - // The widest valid body, per variant. - for _, v := range variants { - prog = append(prog, gatedStep(stepCreateMaximal, v)) - } // One update per writable field, capped. for i, name := range fieldNames(fields) { if i == maxUpdateFields { @@ -166,11 +162,18 @@ func buildProgram(createBody *specmodel.Schema, gates []Gate, variants []Variant // prose hypotheses the gate loop did not already cover. prog = append(prog, perValueSteps(gates, hyps)...) - // Teardown. - prog = append(prog, - Step{Kind: stepDeleteWithConfirmation}, - Step{Kind: stepCleanupDelete}, - ) + // Teardown, and the widest valid body inside it. + // + // The maximal create makes a second object and deletes it again, so it + // runs once the first is gone: an API that keys an object on fields the + // two bodies share — both are synthesised from the same document — + // answers the second create with a conflict, and a conflict says nothing + // about how wide a valid body is. + prog = append(prog, Step{Kind: stepDeleteWithConfirmation}) + for _, v := range variants { + prog = append(prog, gatedStep(stepCreateMaximal, v)) + } + prog = append(prog, Step{Kind: stepCleanupDelete}) return prog } diff --git a/internal/audit/strategy/strategy_test.go b/internal/audit/strategy/strategy_test.go index 176f263..52ee43b 100644 --- a/internal/audit/strategy/strategy_test.go +++ b/internal/audit/strategy/strategy_test.go @@ -749,3 +749,35 @@ func TestCompileErrors(t *testing.T) { t.Fatal("resource with no create body should error") } } + +// TestUnit_Strategy_TheMaximalCreateFollowsTheDelete pins the one ordering the +// maximal create depends on. +// +// It makes a second object and deletes it again, so the first has to be gone +// first: an API that keys an object on fields both bodies carry — and both are +// synthesised from the same document — answers the second create with a +// conflict, and a conflict says nothing about how wide a valid body is. +func TestUnit_Strategy_TheMaximalCreateFollowsTheDelete(t *testing.T) { + s := compile(t, flatSpec, "widget", defaultCfg()) + + posOf := func(kind string) int { + for i, st := range s.Program { + if string(st.Kind) == kind { + return i + } + } + t.Fatalf("the program has no %s step: %+v", kind, s.Program) + return -1 + } + + create := posOf("createMinimal") + del := posOf("deleteWithConfirmation") + maximal := posOf("createMaximal") + cleanup := posOf("cleanupDelete") + + if !(create < del && del < maximal && maximal < cleanup) { + t.Errorf("program order is createMinimal=%d delete=%d createMaximal=%d cleanup=%d; "+ + "the maximal create must sit between the delete and the cleanup", + create, del, maximal, cleanup) + } +} From bb113670627ad617d41f71627dc387d5d7d6bdda Mon Sep 17 00:00:00 2001 From: ShocOne <62835948+ShocOne@users.noreply.github.com> Date: Wed, 26 Aug 2026 13:32:02 +0100 Subject: [PATCH 3/3] docs: the handoff says how the acceptance suite is run and where it stands The previous version opened by saying no audit had ever run against any pilot, which stopped being true some time ago. It described the coverage and correctness split, and the job is now narrower than that: make the generated acceptance tests pass, measured by the suite and by nothing else. What a reader needs and could not find anywhere: which script is the benchmark, what the loop script does, where the credential lives, which files each generated resource carries and which of them are replayed rather than derived, and where the audit writes what it learned. The score and the failure split are here because they are the state, not a measurement of the toolkit: counts of what the toolkit emits and refuses stay in docs/emittance_tracker.md. Co-Authored-By: Claude Opus 5 (1M context) --- handoff.md | 409 +++++++++++++++++++++++++++-------------------------- 1 file changed, 209 insertions(+), 200 deletions(-) diff --git a/handoff.md b/handoff.md index ac70319..92bc54b 100644 --- a/handoff.md +++ b/handoff.md @@ -1,257 +1,266 @@ # Handoff -Where the work stands, what it actually achieved, and what is left. +## The job, and the only measure of it -Every number here is reproducible — see [Verifying these numbers](#verifying-these-numbers). -Counts of what the toolkit emits and refuses are not repeated here; -`docs/emittance_tracker.md` is the one place they live. +Make the 35 generated ThousandEyes resource acceptance tests pass. A test +passes when, and only when, this says so: + +```sh +sh '/Users/dafyddwatkins/localtesting/thousandeyes/tf_acceptance_tests.sh' +``` + +Nothing else counts. Audit observation counts, correction counts, attribute +counts, `postcheck`, `make check` — all of these have moved a great deal while +the benchmark stayed at zero. Run the benchmark, quote its number, and treat a +change that does not move it as a change that did nothing. + +**Current score: 1 passed, 34 failed.** It was 0/35 at the start of this work. --- -## Read this first +## The test harness, in full -The distinction this document turns on: **coverage** is how much of a document -generates at all. **Correctness** is whether what generates matches how the API -really behaves — what `docs/mapping.md` specifies and what `README.md` calls -"the work in progress". They are not the same thing. +### Where everything lives -**Correctness work has now started, and the offline half is largely done.** -Constraint keywords parse, bounds and patterns become validators, `format: -password` and `writeOnly` become `Sensitive`, `deprecated` becomes a -`DeprecationMessage`, and a documented default fills the response. Every one of -those was zero when this document was first written. +| What | Path | +|---|---| +| Toolkit (this repo) | `/Users/dafyddwatkins/GitHub/terraform/terraform-plugin-framework-codegen` | +| Generated provider under test | `/Users/dafyddwatkins/GitHub/terraform/scratch/gen/thousandeyes` | +| Built CLI | `/Users/dafyddwatkins/GitHub/terraform/scratch/bin/tfpfgen` | +| Test scripts | `/Users/dafyddwatkins/localtesting/thousandeyes/` | -**The half that needs a live API has not started, and cannot yet.** No audit has -ever run against any of the three pilots: all three revised specs contain zero -`x-tfpfgen-*` extensions, and no scratch tree has an `audit/` or -`spec/corrections/` directory. Every shape that depends on an observation — -server defaults, immutability, normalisation, conditional validity — is -unexercised, and the generated config validators along with them. +Other pilot trees sit beside the ThousandEyes one: `github`, `jamfpro`, +`credentials`. Only ThousandEyes has a live token and an audit. -### The presence census +### The scripts -The measure of the remaining gap. `docs/contract.md` states the intent: +`tf_set_env_var.sh` — the one place the lab credential lives. Every other +script sources it. It exports `THOUSANDEYES_API_TOKEN` (what the generated +provider reads), `TFPFGEN_AUTH_TOKEN` (what `tfpfgen audit run` reads — the +toolkit's fixed secret contract) and `TE_PROVIDER_DIR`. -> `Optional` alone is the rare one. Most APIs answer with a value for every -> field they accept … emitting it as `Optional` alone gives the practitioner a -> perpetual diff. +`tf_acceptance_tests.sh [filter]` — **the benchmark**. Discovers every package +under `internal/services` holding a `TestAcc`, runs them with `TF_ACC=1`, and +prints a summary ending `N passed, M failed, K skipped`. The optional argument +is a case-insensitive substring matched against package paths: -Counting `computed_optional_required` across every attribute tree of all three -pilots, at `30b23ac` on 2026-08-25: +```sh +sh tf_acceptance_tests.sh # all 35 +sh tf_acceptance_tests.sh tags/v1/tag # one resource, ~7s +sh tf_acceptance_tests.sh tests/v1 # a group +``` -| | optional | computed_optional | ratio | -|---|---|---|---| -| when first measured | 2,221 | 64 | 35× | -| after the coverage work | 3,216 | 65 | 49× | -| now | 10,574 | 670 | **15.8×** | +Knobs: `TE_PARALLEL` (default 1 — both `-p` and `-parallel`, so nothing runs +concurrently against the live tenant), `TE_TIMEOUT` (default 120m, the whole +run's `go test` timeout, not per test). -The absolute figures are not comparable across rows — union variants brought -each branch's own fields into the tree, so there are far more attributes to -count than there were. The ratio is the figure that means something, and it -moved the right way for the first time: `x-tfpfgen-server-default` is still the -thing that would close it, and only an audit produces one. +Logs land at `/Users/dafyddwatkins/localtesting/thousandeyes/acceptance-.log`. ---- +`tf_remediation_loop.sh [filter]` — the full loop: build tfpfgen → probe the +live API → compile and accept corrections → regenerate → run the benchmark, +repeated until the pass count stops moving. Knobs: `TE_LOOP_MAX` (default 5), +`TE_LOOP_AUDIT` (`0` reuses the last probe findings — much faster when only +generator code changed), `TE_LOOP_ACCEPT` (`0` stops and leaves proposals for +review). It ends by ranking the distinct `Error:` reasons from the last run, +which is where the next change comes from. Per-phase logs go to a timestamped +`remediation-/` directory. -## What merged +**`TE_LOOP_ACCEPT=1` crosses the correction gate automatically.** That gate is +deliberate — accepting a correction is a human decision the pipeline exists to +enforce. The loop crosses it on purpose; run with `TE_LOOP_ACCEPT=0` first if +you want to read what the probe wants to change. -`#72`–`#86` are the coverage work, described in the git history. Since then: +### What each generated resource carries -| PR | What | +For `` under `internal/services/resources//v1//`: + +| File | Purpose | |---|---| -| #87 | `docs/mapping.md` committed — the specification for all correctness work | -| #88 | Integer path parameters parsed with a diagnostic | -| #89 | List-resource test content type | -| #90 | List-resource addressing schema | -| #91 | Schema constraint keywords parse (`maxLength`, `uniqueItems`, `minimum`, …) | -| #92 | Update the ThousandEyes test document | -| #93 | `Sensitive` and `DeprecationMessage` emitted | -| #94 | Constraint validators emitted from declared bounds | -| #95 | Point the GitHub test document at an immutable ref | -| #96 | A documented default fills the response | -| #97 | `prune.go` decomposed | -| #98 | Comment sweep | -| #99 | A list resource requires a resource to match | -| #100 | Resource identity schema | -| #101 | Fixtures respect `format` | -| #102 | Generated tests assert what the provider actually produces | -| #103 | `docs/mapping.md` gains the entity operation sets | -| #104 | A lone write is an invocation, whichever method spells it | -| #105 | A datasource filters on the fields of the objects it lists | -| #106 | A filter survives only where the field it selects on does | -| #107 | `docs/emittance_tracker.md` — counts live in one file | -| #108 | A list result names the object by the key the resource is addressed by | -| #109 | A union becomes one attribute per variant where nothing writes it | -| #110 | `CLAUDE.md` and `README.md` restated against the tree they describe | -| #112 | Update the ThousandEyes test document again | -| #113 | The test documents named `vendor_openapi_specs` rather than *corpus* | -| #114 | Those documents committed and embedded, retiring the fetch-and-pin scheme | +| `resource_acceptance_test.go` | the live lifecycle: **step 1** apply minimal, **step 2** import + `ImportStateVerify`, **step 3** apply maximal | +| `tests/terraform/acceptance/resource_{minimal,maximal}.tf` | what the live steps apply — **built from recorded bodies** | +| `tests/terraform/unit/resource_{minimal,maximal}.tf` | what the unit tests apply — **still derived from the document** | +| `tests/responses/resource_{minimal,maximal}.json` | wire JSON the mocks answer with | +| `mocks/responders.go` | httpmock responders built from those | +| `crud.go`, `state.go`, `construct.go`, `model.go`, `resource.go` | the provider itself | ---- +The acceptance and unit fixtures diverge on purpose. Unit configs must keep +matching their mocks, which are built from the same derivation; acceptance +configs replay what the API accepted and carry a per-run random suffix. -## Where correctness stands - -`docs/mapping.md` lists thirteen API behaviours and the shape each demands. -**Detection** is whether the audit can observe the behaviour; **expression** is -whether the generator can emit the shape. Measured in the generated trees, not -in this repo's source — an emitter builds most of what it emits. - -| # | Behaviour | Detect | Express | -|---|---|---|---| -| 1 | Accepted on write, never returned | ✗ no observation kind | ✗ `WriteOnly` never emitted | -| 2 | Never accepted, always returned | ~ `writable=false`, `serverForced`, `volatile` | ~ Computed yes; `UseStateForUnknown` still only on `id` | -| 3 | Optional in, always returned | ✓ `serverDefault` | ~ a *documented* default now fills the response; an observed one needs the audit | -| 4 | Returned obfuscated (`****`) | ✗ misreads as `serverForced` | ✓ `Sensitive` emitted from `format: password` / `writeOnly` | -| 5 | Valid only when a sibling equals a value | ✓ `validWhen` | ~ emitter exists; **no tree contains one**, because no audit has run | -| 6 | Settable at create, refused after | ✓ `immutable` | ~ `RequiresReplace` yes; `…IfConfigured` no | -| 7 | Rejected at create, settable on update | ✗ | ✗ `construct.go` still discards `isCreate` | -| 8 | Echoed back semantically equivalent | ✓ `normalisation` | ✗ no custom type, no `SemanticEquals` | -| 9 | Silently clamped or truncated | ~ enums only | ✓ bounds, lengths and patterns become validators | -| 10 | Omitted → returns `""`/`[]`/`0` | ~ lands as `serverDefault` | ✗ no policy either way | -| 11 | Collection returned in arbitrary order | ✗ | ✗ `uniqueItems` now parses, but `SetAttribute` is never emitted | -| 12 | Collection returns server-injected members | ✗ | ✗ | -| 13 | Field carries one of several object shapes | n/a structural | ~ one attribute per variant where nothing writes it; a writable union is refused | - -Emitted symbol counts across the three trees at `30b23ac`, 2026-08-25, every -one of which was zero when this document was first written: -`Sensitive` 53, `DeprecationMessage` 171, `int64validator.Between` 180, -`UTF8LengthBetween` 42, `LengthAtLeast`/`LengthAtMost` 138, `RegexMatches` 21, -`Default` 18. - -Still zero, and each is a row above: `WriteOnly`, `SetAttribute`, -`SetNestedAttribute`, `SemanticEquals`, `RequiresReplaceIfConfigured`, and any -config validator at all. - -`specmodel.Schema` now parses `writeOnly`, `deprecated`, `uniqueItems`, -`maxLength`, `minLength`, `maxItems` and `minItems`. Only `nullable` remains -unparsed. +### The audit artifacts ---- +Under the provider tree: -## Outstanding coverage work - -Refusals grouped by what the reason says, across all three pilots. The stage -split and the totals are in `docs/emittance_tracker.md`. - -| Family | Share | Note | -|---|---|---| -| SDK model lacks the accessor | 729 | The biggest by far, and the one to characterise next. Mostly fields the generated model genuinely lacks rather than a naming bug. | -| Object with no declared shape | 139 | `additionalProperties: true`, or neither properties nor `additionalProperties`. The vendor documented nothing — arguably a vendor-facing report rather than codegen work. | -| Singleton at a fixed path with no operation set | 125 | One object at a fixed path that fits no operation set. | -| Collection shape unsupported | 105 | Arrays of arrays, maps of objects. | -| Read/write type mismatch | 89 | What survives after #85. | -| Nothing survives to read back or send | 88 | Every field of the entity was refused, so the entity goes too. | -| Terraform reserved name at a schema root | 17 | | - -Two families from the previous handoff have all but closed: the path-parameter -type mismatch is down from 250 refusals to 1 (#88), and union refusals from 90 -to 13 (#109) — eleven of those thirteen a branch referencing no component. - -### Gap 2b — file transfer - -`multipart/form-data`, untouched. Agreed shape is `source` and -`content_base64`, mutually exclusive via `resourcevalidator.Conflicting`, with a -computed `content_base64` for downloads. Needs five things, which is why it was -deferred: `multipart/form-data` parsing in `specmodel`; an IR flag for "this -operation takes a file"; a construct idiom that is `AddOrReplacePart(name, -contentType, content)` rather than field setters; the two attributes and their -validator; and **a request adapter reachable from the resource** — -`MultipartBody.SetRequestAdapter` needs one and generated services receive -`*sdk.APIClient`, not the adapter. +| Path | What | +|---|---| +| `audit/observations/.observations.json` | one fact per property | +| `audit/bodies/.bodies.json` | **the accepted create bodies** — request, response, status | +| `audit/inputs.json` | operator-supplied values the probe cannot invent (authored) | +| `spec/corrections/*.correction.json` | accepted corrections (authored) | +| `spec/corrections/proposed/` | awaiting a human decision | +| `spec/revised.yaml` | generated — never hand-edit | --- -## Owed by the repository owner before work can start +## The architecture -`CLAUDE.md` makes every domain term owner-approved. These block their gaps: +The document is a hypothesis; the API is the authority. -- **The undeclared-response-schema observation** — its kind, its `x-tfpfgen-*` - key, and whether it is eligible for `audit.auto_accept`. The vendor declares a - 200 with no schema, so there is nothing to map into state; the agreed approach - is an observation recording the shape the API actually returned. Also needs - quirkserver ground truth: a shape whose document declares no response schema - while the server returns one. -- **`mapping.md` row 1** — `WriteOnly` plus an `..._version` Int64 trigger would - be the first generated attribute with no wire counterpart. Name and suffix. -- **`mapping.md` row 8** — `x-tfpfgen-normalisation` and its value set. - `internal/spec/revise/compile.go:116` still refuses for want of it. +``` +OpenAPI doc ──> specmodel ──> IR ──> sdkbind ──> emit ──> provider tree + ^ | + | v + corrections <── revise <── observations <── audit (live probe) + + bodies +``` -Settled since this list was written: variant sub-attribute naming (#109, now in -the glossary as **variant attribute**), and `docs/mapping.md` is committed. +Presence (`required` / `optional` / `computed`) is corrected into the document +and re-derived. **Values are not.** Values come from `audit/bodies/` and are +replayed directly into acceptance fixtures. That split is the point: deriving +values again from the document is what produced a year of one-field-at-a-time +failures (`icon`, `match_type`, `filters` were all the same bug). + +### Key code + +| Concern | Where | +|---|---| +| Additive minimal search (add a field until 2xx) | `internal/audit/run/steps_create.go: searchMinimal` | +| Subtractive maximal reduction (drop until 2xx) | `internal/audit/run/steps_create.go: reduceMaximal` | +| Refusal grammar (what a 4xx names) | `internal/audit/run/adjust.go: classifyRefusal` | +| Recorded bodies artifact | `internal/audit/observe/bodies.go` | +| Replaying a body into a fixture | `internal/fixtures/fixtures.go: FromAcceptedBody` | +| Per-run unique names | `internal/fixtures/fixtures.go: WithRunSuffix`, `RunSuffixBlock` | +| Acceptance vs unit split | `internal/emit/render_fixtures.go: resourceFixtures` | +| Probe step ordering | `internal/audit/strategy/program.go: buildProgram` | +| Presence rule | `internal/intermediate_representation/attributes.go` (~line 490) | --- -## Lessons that change how to work on this +## What has merged -**Check what the SDK already decided before designing from the document.** -Three premise failures in one session, all the same shape: +| PR | What | +|---|---| +| #115 | a nested attribute is held as a value that can be unknown | +| #116 | the audit reads the refusals it is given | +| #117 | a created resource answers with what names it (identity, create-response id, examples, safe literals) | +| #118 | an entity names the property that identifies it (`x-tfpfgen-identifier-property`) | +| #119 | an acceptance test replays a request the API took | -- *Typed maps* — the plan assumed the SDK carries a Go map. kiota emits no - `map[string]string` at all; it generates a model whose only field is - `additionalData map[string]any`. -- *Discriminated unions* — the plan assumed documents declare discriminators. - GitHub, which owned almost every union refusal, declares none. -- *Merging object unions* — argued for on the grounds that branches need naming - and reads are ambiguous. Both false: kiota names every branch and exactly one - accessor is non-nil. Building it made GitHub's refusals sharply worse. +**Open PR** — the branch this handoff is on: the maximal create ordering fix, +plus the quirkserver's `NamesRefusedFieldInProse`. -In each case a five-minute `grep` of the generated SDK would have prevented -hours. Measure the pilots before designing, not after. +--- -**Measure at the layer the claim is about.** An earlier revision of this file -reported ten framework symbols as never emitted. Four of them were being emitted -at the time. The census had been taken by grepping this repo for the symbol, and -`internal/emit/render_constraints.go` spells a pair of bounds as -`fmt.Sprintf("%sBetween(%v, %v)", …)` — so the string never appears here and -always appears in the output. Grep the generated tree. +## Where the 34 failures actually are -**`postcheck` catches what unit tests do not.** Two bugs in #85 produced -generated Go that did not compile, and neither would have surfaced in the -toolkit's own suite: construction typed a slice from the *getter* while filling -it with the *constructor's* values; and a nested block with no writable children -rendered a loop declaring an index nothing read. +``` +33 fail at Step 1/3 (create) + 1 fail at Step 3/3 -**Refusal counts going *up* can be correct.** One misleading entity-level -refusal becoming several accurate field-level ones raises the total and improves -the toolkit. Read the reasons, not just the total. +30 Error: Create failed (HTTP 4xx) + 4 Error: Provider produced inconsistent result after apply + 1 Error: Invalid id + 1 Error: Create failed: the request never completed +``` -**Rebase rather than merge `main` into these branches.** #83 and #85 were merged -the other way and both broke `main`. +**The score tracks recorded-body coverage.** Only four entities have a body +recorded — `tag`, `credential`, `dashboard`, `connectors_generic` — because +only four got through the probe. The other 31 still have document-derived +configs, which is why they fail at step 1 with a 4xx. + +So the single highest-value work is **getting more entities through the +probe**. Everything else is downstream of that. + +### Next steps, ranked + +1. **Unblock the probe.** 30 entities blocked in the last run. Read the reasons + in the audit output — they group into: parent objects that cannot be + re-created, creates the search could not heal, and a tenant ceiling on + `/templates`. Each group unblocked is several tests. +2. **Populate `audit/inputs.json`.** Some entities need real tenant values no + generator can invent: a reachable stream endpoint URL, an agent id + (`tests_ftp_server`), a dashboard id (`dashboard_snapshot`), a discriminator + `type` for `connectors_generic` / `operations_webhook`. +3. **The four inconsistent-result failures.** A value sent and echoed back + differently. `FromAcceptedBody` already drops what is never returned; this is + the narrower case of a value the API rewrites. +4. **`Invalid id` / `request never completed`** — one each, `templates_sharing_setting` + (a singleton) and `stream` (`lastSuccess`/`lastFailure` declared `int64`, the + API answers RFC 3339). --- -## Verifying these numbers +## Blockers that are not code -Five local gates, all of which CI also runs. `make check` is the first four: +**`account_group` cannot pass.** A leaked object permanently holds the fixture +name and the API refuses to delete it: -```sh -make check # fmt, build, vet, coverage, hygiene -golangci-lint run # make check leaves this to CI +``` +DELETE /v7/account-groups/281474976717041 +400 "Unable to delete accounts outside your organization." ``` -Then the loop that matters, into `/Users/dafyddwatkins/GitHub/terraform/scratch/gen`: +It needs org-admin or ThousandEyes support. The per-run random suffix (#119) +means new leaks will not recur, but this one predates it. -```sh -go build -o …/scratch/bin/tfpfgen ./cmd/tfpfgen -cd …/scratch/gen/ -tfpfgen provider generate # postcheck: go mod tidy, go build, go vet -tfpfgen provider verify # must report no drift -``` +**Import is flaky.** `tag` failed once at step 2 with `Read failed (HTTP 403)` +and passed on a re-run with no change — read-after-write lag, with ThousandEyes +answering 403 where 404 is expected. Any full-suite number is unreliable until +the retry predicate treats that as lag. Re-run a single failure before +believing it. + +**A full suite run is slow.** Serial by design (`TE_PARALLEL=1`). One earlier +run had a single resource burn 1802s on a create timeout. Prefer filtered runs +while iterating. + +--- -The emitted-symbol census, which is a fact about the generated tree: +## Lessons that would have saved hours + +**Run the benchmark.** Every proxy metric moved while it sat at zero. If a +change cannot be shown to move the pass count, say so plainly rather than +reporting the proxy. + +**Verify a diagnosis before acting on it.** Three claims in this work were +asserted confidently and were wrong: that a live minimal object was racing the +maximal create (it was — but only under the *strategy* program, not the plan +path that was read first); that the recipe carrying a stale body caused the +maximal failures (it did not — two tests written to prove it passed without the +fix); and that a regression was caused by a code change when it was a local DNS +failure. Instrument and look. One `fmt.Fprintf` to stderr settled in one run +what an hour of reading did not. + +**Fix a fact in the layer that owns it.** Two workarounds were written into the +emitter for facts that belong to the probe — sending `x-tfpfgen-server-default` +as an input value, and stopping the state mapper nulling any optional +attribute. Both were reverted in #119. If a fact about the API has nowhere to +live, that is a missing observation kind, not a licence to put policy in the +generator. + +**Naming is owner-owned.** A new observation kind or `x-tfpfgen-*` key needs +the repository owner to approve the name before it is coined, and the decision +recorded in `docs/glossary.md`. `identifierProperty` went through that; the +"accepted on write, never returned" fact (`docs/mapping.md` row 1) still needs +it. + +--- + +## Verifying the toolkit itself ```sh -grep -rc 'Sensitive:' /internal/services | grep -v ':0$' +make check # fmt, build, vet, coverage gate, hygiene gate +golangci-lint run # make check leaves this to CI ``` -The presence census: +Coverage gate: 90% total, 80% per package under `internal/`. Currently 90.7%. + +Regenerating a pilot tree: ```sh -tfpfgen provider generate --print-ir > ir.json -# count computed_optional_required across every attribute tree +go build -o ../scratch/bin/tfpfgen ./cmd/tfpfgen +cd ../scratch/gen/thousandeyes +tfpfgen provider generate # postcheck: go mod tidy, go build, go vet ``` -**The report is the acceptance test.** Each piece of work should name the count -it expects to move, and the before/after `unsupported.json` totals should show -it moved by that much and nothing else changed. That is a stronger gate than any -unit test here, because it measures three real documents. +Claims about generated output are verified against a generated tree, never by +grepping this repo for a symbol — an emitter builds most of what it emits.