docs: adapt the comment style guide to this repo, and sweep

docs/comment-style.md

@@ -2,11 +2,15 @@
 
 Comments in this repo state **what** and **why**. Nothing else.
 
-They are not a record of how the code was arrived at. The investigation belongs in
-`docs/backends.md` and in commit messages, which are where this project already
-keeps measured evidence. A comment that retells the debugging story buries the one
-or two sentences a reader needs, and it ages badly — the narrative stays after the
-finding it describes has been superseded.
+They are not a record of how the code was arrived at. The investigation belongs
+in the pull request body and in commit messages, which are where this project
+keeps measured evidence and where a measurement carries a date. A comment that
+retells the debugging story buries the one or two sentences a reader needs, and
+it ages badly — the narrative stays after the finding it describes has been
+superseded.
+
+`CLAUDE.md` states the same rule in short form; this document is the long form,
+and the two do not disagree.
 
 ## The rule
 
@@ -31,76 +35,81 @@ statement whose reason is local to that statement.
 
 These are facts, and they are the point of the comment.
 
-- **External-system behaviour, as a standing property.** "HNS reads
-  `Ipams[].Subnets[]` back as a flat `Subnets[]`." "`query user` answers
-  *No User exists for \** on a guest that plainly has a console session."
-- **Accepted and rejected value sets**, and the hardware or build-number
-  qualifiers that scope a claim: "SecureNestedPaging requires AMD SEV-SNP",
-  "measured on build 26200".
-- **Raw evidence quoted inline** — HRESULTs (`0x8007139F`), SDDL strings
-  (`D:P(A;;FA;;;BA)(A;;FA;;;SY)`), Winsock error names. They are what makes the
-  surrounding claim checkable.
+- **Generator and API behaviour, as a standing property.** "kiota models an
+  object union as a composed type with one named accessor per branch." "A
+  document that declares nothing required in its responses sends every writable
+  attribute to plain `Optional`."
+- **Accepted and rejected value sets**, and the backend or version qualifiers
+  that scope a claim: "`list/schema` attribute types declare no `Sensitive`
+  field", "measured against terraform-plugin-framework v1.19.0".
+- **Raw evidence quoted inline** — an SDK accessor spelling
+  (`GetPasswordEscaped`), a refusal reason as the report prints it, an HTTP
+  status, a framework type name. They are what makes the surrounding claim
+  checkable.
 - **Indented tabular blocks.** They render as godoc code blocks and usually carry
-  more than the prose around them. See `internal/hcs/netmodes.go` (mode → HCN
-  network) and `internal/hcs/reconcile.go` (network type → how it reports
-  `SwitchGuid`). Keep the table, rewrite the prose framing it.
-- **Cross-references.** Prefer paths and symbols over URLs: `docs/backends.md`,
-  gate ids (G1, G2, G4), milestone ids (M3, M4), env var names. Keep a gate or
-  milestone id next to its `docs/backends.md` reference so it stays resolvable.
-  This is a preference about *form*, not a licence to delete citations — a
-  pointer into upstream source that lets a reader check a claim
-  (`hcsshim internal/uvm/create_wcow.go`) is evidence and stays. Drop a bare URL
-  only when the sentence still stands without it.
+  more than the prose around them. See `internal/corpus/cache.go` (the cache
+  layout) and `internal/sdkbind/binder_kiota.go` (document shape → builder
+  chain). Keep the table, rewrite the prose framing it.
+- **Cross-references.** Prefer paths and symbols over URLs: `docs/contract.md`,
+  `docs/glossary.md`, `docs/mapping.md`, `x-tfpfgen-*` keys, `TFPFGEN_*`
+  environment variable names, the exit-code contract. Keep a mapping.md row
+  number next to its reference so it stays resolvable. This is a preference
+  about *form*, not a licence to delete citations — a pointer into upstream
+  source that lets a reader check a claim
+  (`terraform-plugin-framework-validators stringvalidator/utf8_length_between.go`)
+  is evidence and stays. Drop a bare URL only when the sentence still stands
+  without it.
 - **Contracts** — locking requirements, `nil, nil` returns, ordering guarantees,
-  what a caller must do first.
-- **Operator-visible risk in acceptance tests** — why a test is opt-in, what it
-  does to the host. State it as a standing risk, not as an anecdote.
+  what a caller must do first, which stage owns a refusal.
+- **Operator-visible risk in credentialed tests** — why a test is opt-in, what it
+  does to somebody's tenant. State it as a standing risk, not as an anecdote.
 
 ## Drop
 
 - **Discovery narrative and post-mortems.** "used to", "we tried", "turned out",
   "it was tried and does NOT work", "that happened and nothing noticed",
-  "for as long as weave asked HNS to…", "which is how X became Y".
-- **Incident statistics and blame.** "one exec in fifteen hundred bound the module
-  bus's port", "it cost an acceptance run", "this gate has already produced two
-  'we tested that' claims that turned out to be testing something else".
+  "which is how X became Y", "had already drifted into a duplicate".
+- **Counts and measurements from a particular run.** "207 losses on one pilot",
+  "the first live run opened fifty-seven", "42 attributes across the three
+  pilots". A measurement belongs in a pull request body, where it is dated.
 - **Retracted conclusions.** State only what is true now. A superseded finding
   kept "because it was load-bearing for a while" is a trap for the next reader.
-- **Roadmap and project commentary in API docs.** Which milestone a refactor lands
+- **Roadmap and project commentary in API docs.** Which tranche a refactor lands
   in is not a property of the function.
-- **Editorial asides.** "the crux of the Windows guest story", "worth having
-  anyway", "the rest is left alone deliberately".
-- **Restatements of the signature.** `// New returns an HCS engine.`,
-  `// Resume resumes a paused VM.`, `// Profiles returns the registry.` Either say
-  something the signature does not, or say nothing.
-- **Porting history**, beyond a single attribution line in a file header.
+- **Editorial asides.** "worth having anyway", "the rest is left alone
+  deliberately", "which is the point".
+- **Restatements of the signature.** `// Prune prunes the bindings.`,
+  `// Load loads a document.`, `// IDs returns the ids.` Either say something the
+  signature does not, or say nothing.
+- **Which pull request changed it**, and how a bug was found. Git history
+  records change; a comment states what is true.
 
 ## Transform, don't delete
 
 Almost every narrative sentence contains a fact. Convert the story into the
 standing property it implies; do not throw the fact out with it.
 
-Before — 14 lines:
+Before — 11 lines:
 
-> installedMatches reports whether the guest already holds exactly this binary,
-> comparing content hashes.
+> joinTreeKeeping is joinTree, and also answers the attributes it kept with no
+> SDK field behind them — the id and the addressing attributes.
 >
-> The obvious check — ask the deployed agent its version and compare — is not
-> enough, and the way it fails is quiet. The version identifies the protocol, so
-> it does not move when the binary changes for any other reason: a bug fix, a
-> dependency bump, a different link flag. Rebuilding the agent for the GUI
-> subsystem changed nothing about the protocol, so a version check reported the
-> guest up to date and left the old binary running, console window and all.
+> Pruning removes their bindings, correctly: no model carries them, because they
+> address the object rather than describe it. But the attribute still reaches the
+> schema, so reporting that removal as something the operator lost is wrong, and
+> it was wrong 207 times on one pilot. This is the only place that knows, because
+> this is the place that decides.
 
-After — 6 lines, same information, no story:
+After — 7 lines, same information, no run-specific count and no editorial:
 
 ```go
-// installedMatches reports whether the guest already holds exactly this binary,
-// comparing content hashes.
+// joinTreeKeeping is joinTree, and also answers the attributes it kept with
+// no SDK field behind them — the id and the addressing attributes.
 //
-// A version comparison is not sufficient: the version identifies the protocol,
-// so it does not move when the binary changes for any other reason — a bug fix,
-// a dependency bump, or a link flag such as -H windowsgui.
+// Pruning removes their bindings correctly: no model carries them, because
+// they address the object rather than describe it. The attribute still
+// reaches the schema, so reporting that removal as a loss would be wrong,
+// and this is the only place that knows which attributes those are.
 ```
 
 ## Section dividers
@@ -134,12 +143,12 @@ package hcs
 
 ## Where the doc goes for commands
 
-- **`internal/command`** — the `XxxCommand` struct carries the prose that explains
-  the feature. Its `Validate` and `Run` methods each carry one to three lines
-  saying what they do and any state precondition they enforce.
 - **`internal/cli`** — user-facing text lives in the cobra `Short`, `Long` and
   `Example` fields, which is the right home for it. Each `newXxxCommand`
-  constructor still carries a one-line Go doc so `go doc` is not blank.
-- **`cmd/hcsspike`** — a lab notebook rather than a library. Its measured verdict
-  tables and pass/fail matrices are the deliverable and stay verbatim. The
-  self-narration around them does not.
+  constructor still carries a one-line Go doc so `go doc` is not blank. A verb's
+  exit-code behaviour belongs here, next to `docs/contract.md`'s table.
+- **`cmd/tfpfgen`** — three lines of substance over `cli.Run`. It needs a package
+  comment and nothing else; everything a reader wants is in `internal/cli`.
+- **`internal/templates`** — a template's comments are emitted into somebody
+  else's repository, where nobody can edit them. They say what the generated
+  code does, never how this toolkit decided to generate it.

internal/config/reference.go

@@ -2,9 +2,9 @@ package config
 
 // docs/config.md is generated, not written: Reference walks the Config
 // struct by reflection, so the reference can only describe keys the decoder
-// actually reads. v1 documented keys that nothing consumed; here a dead key
-// in the docs is structurally impossible, and the bidirectional test over
-// the descriptions map makes a dead description impossible too.
+// actually reads: a documented key nothing consumes is structurally
+// impossible, and the bidirectional test over the descriptions map makes a
+// description with no key impossible too.
 
 import (
 	"fmt"

internal/config/secrets.go

@@ -1,9 +1,9 @@
 package config
 
 // Secret names are fixed by auth role — they are a contract, not
-// configuration. v1 let the workflow and the config each spell secret names
-// and a rename broke the pipeline with an error citing the old spelling;
-// here there is exactly one definition site.
+// configuration, and this is their one definition site. Letting the workflow
+// and the config each spell them means a rename breaks the pipeline with an
+// error citing the spelling that was replaced.
 const (
 	SecretToken         = "TFPFGEN_AUTH_TOKEN"
 	SecretClientID      = "TFPFGEN_AUTH_CLIENT_ID"

internal/config/validate.go

@@ -15,7 +15,7 @@ var dnsLabel = regexp.MustCompile(`^[a-z][a-z0-9-]*[a-z0-9]$`)
 
 // releaseTag matches an exact toolkit release. Branch names — "main"
 // included — are deliberately unmatchable: a pipeline that reinstalls a
-// moving target on every run was one of v1's failure modes.
+// moving target on every run cannot reproduce what it generated.
 var releaseTag = regexp.MustCompile(`^v\d+\.\d+\.\d+$`)
 
 // problems returns every semantic defect at once. An empty slice means the

internal/corpus/corpus.go

@@ -55,9 +55,8 @@ const EnvRequired = "TFPFGEN_CORPUS_REQUIRED"
 // relative default is resolved against the working directory, and `go test`
 // runs every package in its own directory -- so one suite run would scatter a
 // copy of every document through the tree, in package directories no
-// root-anchored .gitignore pattern reaches. v1 learned this the hard way: it
-// put 19 MB of fetched specifications into a commit whose entire purpose was
-// removing generated output from the repository.
+// root-anchored .gitignore pattern reaches, and the documents are megabytes
+// each.
 //
 // A cache belongs where the operating system puts caches. CI overrides it with
 // TFPFGEN_CORPUS_DIR to somewhere its cache action can restore.
@@ -92,9 +91,9 @@ type Pin struct {
 	// MirrorURL holds the exact pinned bytes under our own control, and is
 	// tried first.
 	//
-	// Not a nicety. Several of these upstreams serve "current" rather than a
-	// version, so the pinned bytes become unfetchable the moment the vendor
-	// publishes again, and every pin update is forced on their schedule. A
+	// Several of these upstreams serve "current" rather than a version, so
+	// the pinned bytes become unfetchable the moment the vendor publishes
+	// again, and every pin update is forced on their schedule. A
 	// mirror cannot alter test meaning -- the SHA-256 is checked identically
 	// whichever source answered -- so it is a CDN, not a source of truth.
 	MirrorURL string `json:"mirrorUrl,omitempty"`

internal/emit/render_mapping.go

@@ -364,11 +364,10 @@ func buildCallPlan(call *sdkbind.Call, payloadName string, nodes []node, modelVa
 	for position, p := range call.Params {
 		// The last path parameter addresses the object itself, which is what
 		// the id attribute holds however the API spells the parameter. The
-		// fallback used to need the call to take exactly one parameter, which
-		// is only true of a flat API: /enterprises/{enterprise}/code-security/
-		// configurations/{configuration_id} takes two, and its id was left
-		// matching nothing because the response happened to declare an id of
-		// its own and keep that spelling.
+		// fallback is offered on position rather than on the call taking
+		// exactly one parameter: only a flat API is single-parameter, and
+		// /enterprises/{enterprise}/code-security/configurations/{configuration_id}
+		// takes two while still naming one object.
 		n, err := paramNode(p, nodes, position == len(call.Params)-1)
 		if err != nil {
 			return callPlan{}, err

internal/emit/render_schema.go

@@ -285,9 +285,8 @@ func (sb *schemaBuilder) planModifierLines(n node, indent string) string {
 // The order is the point. The document's sentence is the only human-written
 // text in the whole pipeline and it is what a practitioner actually needs;
 // the inferred facts qualify it. Where the document says nothing — and real
-// ones often do not, one pilot annotating 12% of its properties against
-// another's 52% — the wire property name stands in, which is no worse than
-// what was rendered before and no better.
+// ones routinely annotate only a fraction of their properties — the wire
+// property name stands in.
 func attributeDescription(a ir.Attribute) string {
 	var parts []string
 	if declared := strings.TrimSpace(a.Description); declared != "" {
@@ -329,14 +328,12 @@ type modelDecl struct {
 //
 // The short spelling — the type prefix, the attribute's Go name, "Model" —
 // is what a tree gets whenever it is the only nesting site claiming it. Two
-// sites claiming one spelling used to be a hard error naming both paths and
-// telling the operator to "rename one in the document", which is not
-// something a vendor's document will do: a real document nests an object of
-// one name at two depths of one entity, and that single collision aborted
-// every resource the provider had. Every claimant of a contested spelling is
-// therefore qualified by its ancestor path instead, and an uncontested one is
-// left exactly as it was — so qualification shows up only where a collision
-// made it necessary.
+// sites claiming one spelling cannot be refused: a real document nests an
+// object of one name at two depths of one entity, and a vendor will not
+// rename it to suit a generator. Every claimant of a contested spelling is
+// qualified by its ancestor path instead, and an uncontested one is left
+// short — so qualification shows up only where a collision made it
+// necessary.
 //
 // Which sites are contested is decided from the whole tree before any name
 // is handed out, so the answer never depends on the order names are asked

internal/emit/schema_type.go

@@ -11,14 +11,10 @@ import (
 // that follows from "this attribute is a list of objects" or "this
 // attribute is a bool", resolved once and read as fields.
 //
-// It exists because those names used to live in six switches on the same
-// (Kind, ElementType, Nested != nil) triple, spread across four files, each
-// returning a bare string. Nothing tied them together, so adding an
-// attribute type meant editing six places and hoping none was missed, and
-// the compiler could not help — every one of them returned string. One of
-// the six had already drifted into a duplicate: frameworkValueType in
-// render_datasource.go answered exactly what fieldType answered for a
-// scalar, spelled separately.
+// One record rather than a switch per name: every one of these follows from
+// the same (Kind, ElementType, Nested != nil) triple, and answering them
+// separately means a new attribute type has to be added everywhere at once,
+// with the compiler unable to help because each answer is a bare string.
 //
 // Field names are terraform-plugin-codegen-spec's, so this record and the
 // specification that could describe its output agree: ValueType from

internal/emit/services.go

@@ -320,10 +320,10 @@ func rawFile(outPath, source string, content []byte) File {
 // requires every resource and datasource to have an id; the API need not
 // agree. A read response that names its key something other than id, or omits
 // it altogether and leaves it in the URL, is an ordinary REST shape, and both
-// are common in real documents. Dropping the id node on those used to abort
-// the whole provider from render_mapping's path-parameter lookup. The id's
-// value comes from the path parameter and from whatever the create response
-// carries, neither of which needs a read binding.
+// are common in real documents. The id node has to survive them both:
+// render_mapping's path-parameter lookup reads it, and dropping it aborts the
+// whole provider. Its value comes from the path parameter and from whatever
+// the create response carries, neither of which needs a read binding.
 type node struct {
 	attr     ir.Attribute
 	fb       *sdkbind.FieldBinding
@@ -341,11 +341,10 @@ func joinTree(tree *ir.AttributeTree, fbs []sdkbind.FieldBinding, addressing ...
 // joinTreeKeeping is joinTree, and also answers the attributes it kept
 // with no SDK field behind them — the id and the addressing attributes.
 //
-// Pruning removes their bindings, correctly: no model carries them, because
-// they address the object rather than describe it. But the attribute still
-// reaches the schema, so reporting that removal as something the operator
-// lost is wrong, and it was wrong 207 times on one pilot. This is the only
-// place that knows, because this is the place that decides.
+// Pruning removes their bindings correctly: no model carries them, because
+// they address the object rather than describe it. The attribute still
+// reaches the schema, so reporting that removal as a loss would be wrong,
+// and this is the only place that knows which attributes those are.
 func joinTreeKeeping(tree *ir.AttributeTree, fbs []sdkbind.FieldBinding, addressing ...map[string]bool) ([]node, []string) {
 	names := map[string]bool{idAttributeName: true}
 	for _, set := range addressing {