identity golden: the 704 cohort identities #699 unpinned should render from estate-gen output, or the drop should be ruled

[lex00]

What

PR #929 (#699) retires the 31 committed estate-gen cohorts; they now render into a temp dir at run time. The identity golden moved with them: TestIdentityGolden went from 1810 rendered identities across 654 directories to 1106 across 623, a 704-instance drop, all of it cohort identities that were pinned by value and are pinned nowhere now.

The cohorts still render (TestGenerateCohortsRendersTheWholeRoster) and their declared types still match the recorded roster, but the rendered identity of each cohort instance, the thing the golden exists to hold by value, is no longer checked.

Do

Decide whether the golden should render the cohorts from estate-gen output as part of its walk (generate into a temp dir inside the test, render, compare against the committed golden file), restoring the 704 pins without recommitting the trees. If yes: one unit, internal/live/check plus the three-layer pin move back up, with the cost of the generation step stated. If no: say so here and close, so the next reader of HANDOFF's 1106 figure knows the drop was chosen.

Refs #699, #929.

[lex00]

Ruling requested, two options, no preference stated here. Context: #929 (#699) retired the 31 committed estate-gen cohorts; the identity golden dropped from 1810 pinned identities across 654 directories to 1106 across 623, and the 704 cohort identities are pinned by value nowhere today (they render at test time in TestGenerateCohortsRendersTheWholeRoster, which checks declared types against the roster, not rendered identities).

Option 1: the golden renders the cohorts. TestIdentityGolden generates the 31 cohorts into a temp dir inside the test (the estate-gen -all path #929 built), renders them, and compares against a committed golden section for them. Restores the 704 by-value pins without recommitting the trees. Cost: the generation step inside a test that already takes several seconds (estate-gen -all rendered all 31 in about 13s in #929's run), the three-layer pin move back up to about 1810/654, and HANDOFF's figure with it.

Option 2: rule the drop as chosen. The cohorts are generated inputs, their identities are a function of the generator, and the generator's own tests plus the type-roster check are the guard; the golden pins hand-written fixtures only. Cost: 704 fewer by-value identity pins, and the rule "any worker adding a .tf fixture directory must move all three layers" gets a stated exception for generated cohorts. One sentence in HANDOFF's guard table saying so, so the 1106 figure reads as a decision rather than a loss.

Whichever is chosen, the change is one unit. Nothing else waits on it.

[lex00]

Recommendation, so this is a decision rather than an open question. Option 1, the golden renders the cohorts.

The reason is what the golden is for. Its guard-table line says it pins rendered identities by value, and the rule around it is that a worker adding a fixture directory moves all three layers. The 704 identities #929 unpinned are still rendered by the product on every run of those cohorts; what changed is only that the fixtures are generated rather than committed. Nothing about the risk moved, so the pin should not have moved either. The type-roster check that replaced it answers a different question: which types a cohort declares, not what identity each instance renders.

Cost is real but bounded: estate-gen -all rendered all 31 cohorts in about 13 seconds in #929's own run, inside a test that already takes several seconds, and the work is a temp dir plus a compare. That is cheaper than the class of defect the golden exists to catch, which is a rendered identity changing silently.

Option 2 is defensible if the generated cohorts are considered inputs rather than fixtures, but then HANDOFF's guard table needs the exception written down, and the 1106 figure needs a sentence saying it is a choice. Leaving it unstated is the one outcome to avoid, because the next reader sees a number that dropped by 704 with no reason attached.

Either way it is one unit and nothing waits on it.

[lex00]

Option 1 is implemented in #997 and awaits the ruling. Merging it is the ruling; closing it chooses option 2.

TestIdentityGolden renders the 31 cohorts into t.TempDir() with flocitest.GenerateCohorts (the estate-gen -all path #929 built, reused rather than duplicated) and pins them by value in a delimited section labelled <cohorts>/<name>. Measured: 1108 -> 1812 rendered identities across 624 -> 655 directories, 704 added, 0 changed, 0 removed, with the committed-fixture half of the regenerated file byte-identical to the whole of the previous one.

Cost, measured with and without the change on the same machine: TestIdentityGolden 0.72s -> 13.79s, and the whole internal/live/check package 3.91s -> 43.74s. The separable 25.6s of that is a second render in TestIdentityGoldenCohortsAreDeterministic, which proves the generator's determinism rather than any identity and can be dropped without unpinning a row. The other cost is a prerequisite rather than a clock: this test needed no external tool before and now needs go, terraform and the pinned provider, with a loud opt-out (CHOUDOUFU_GOLDEN_SKIP_COHORTS) that refuses -update and still requires the golden's cohort section to be present.

One finding fell out of it, and it is the concrete case for the option: 703 of the 704 restored rows are byte-identical to what #699 removed, and the 704th is <cohorts>/route53-cloudfront's aws_route53_zone_association.app, whose rendered identity gained :placeholder and whose attributes gained vpc_region=placeholder (estate-gen's OmitIfAbsent force-fill, which the retired committed copy predated; #929 found it and ruled the generator the correct side). That string changed while the cohorts were unpinned, and no by-value instrument in the repository would have shown it.

[lex00]

Ruled: option 1, implemented in #997, which I am arming to merge. The deciding evidence is not the argument I made earlier, it is what implementing it turned up.

Of the 704 identities the cohorts contribute, 703 came back byte-identical to what #699 removed. The 704th did not: route53-cloudfront's aws_route53_zone_association.app gained :placeholder in its rendered identity and vpc_region=placeholder in its attributes, from an estate-gen force-fill the retired committed copy predated. It changed while the cohorts were unpinned, and nothing in the repository would have shown it. That is precisely the defect class the golden exists to catch, occurring inside the window this issue was about, so the question stops being a preference.

The cost is real and larger than my estimate, and it is stated on the PR rather than buried: TestIdentityGolden goes 0.72s to 13.79s, the determinism test adds 25.59s, and ./internal/live/check goes 3.91s to 43.74s. The package also gains a dependency on go, terraform and the pinned provider where it previously needed no external tool; CHOUDOUFU_GOLDEN_SKIP_COHORTS is the loud opt-out, and it refuses -update and still requires the cohort section to be present and non-empty.

This is a maintainer call and I have made it rather than leave the issue open, because it raises pinning rather than lowering a bar and a revert is one commit. If you would rather have option 2, revert #997 and add the sentence to HANDOFF's guard table saying the generated cohorts are deliberately unpinned, so the figure reads as a decision.