Skip to content

Publish every generator's period - #563

Merged
Eli Pinkerton (wallstop) merged 11 commits into
mainfrom
dev/wallstop/session-223
Aug 25, 2026
Merged

Publish every generator's period#563
Eli Pinkerton (wallstop) merged 11 commits into
mainfrom
dev/wallstop/session-223

Conversation

@wallstop

@wallstop Eli Pinkerton (wallstop) commented Aug 25, 2026

Copy link
Copy Markdown
Collaborator

Why: No generator stated its period anywhere a caller could read it, and two of the gates that were supposed to catch drift could not fail.

What:

  • Add RandomGeneratorMetadata.Period, annotate all 20 generators, and gate the docs table against it so the two cannot drift.
  • Rewrite the shipped-analyzer gate to run CI's own byte comparison; a renamed directory, a deleted DLL and a build that stopped copying were all invisible to it.
  • Add a meta-check: every linter needs a self-test that can make it report. Seven are allowlisted against ci: seven linters still have no self-test that can make them report #562.
  • Give lint-changelog.ps1 a -ChangelogPath and ten red halves; it could only ever read one file, which is why it had no self-test.
  • Record the measured relational assignment cost, the generator state widths, and three traps.

Fixes #558


Note

Medium Risk
Touches runtime color threshold semantics and documents RNG period claims consumers may rely on; most other changes are additive metadata and CI/tooling, with moderate blast radius if gates regress.

Overview
Adds RandomGeneratorMetadata.Period (plus PeriodLabel) and annotates every shipped IRandom implementation with either a published period or a measured state-width claim. The random-generators docs table gains a Period column, and test-random-periods.js fails the build if any generator omits a period or if docs and attributes disagree.

ColorQuantization.ToThresholdByte no longer uses a bare floor(cutoff * 255f); it adjusts the candidate so inclusive comparisons against ToNormalized stay correct at float boundaries, with boundary-focused tests including the CI regression case.

Repo hygiene and gates: verify-shipped-analyzers.js now builds into a scratch dir and byte-compares named ANALYZERS DLLs to Runtime/Analyzers (missing dir, missing/stale/unexpected DLLs are explicit failures), with test-verify-shipped-analyzers.js. lint-changelog.ps1 accepts -ChangelogPath and gains a full self-test; test-run-repo-lint.js adds a meta-check that every linter has a reachable self-test (registry files excluded; seven linters still tracked on #562). Contract tests and package.json wire the new scripts. .llm skills/context record related traps (V8 regex, linter coverage scans, Unity test namespace filtering, Debug.LogError cost on relational misses).

Reviewed by Cursor Bugbot for commit 3ce5c95. Bugbot is set up for automated code reviews on this repo. Configure here.

verify-shipped-analyzers.js built each analyzer project INTO Runtime/Analyzers
and diffed the directory against itself before and after. Three paths could not
go red (#558):

* a renamed or removed Runtime/Analyzers made `before` and `after` both empty
  maps, so nothing differed and the check reported success forever;
* `moved` iterated `after.keys()` only, so a DLL present before and absent
  after -- the loudest possible mismatch -- contributed no key and was
  invisible;
* a .csproj edit that stopped the copy made the two sides identical by
  construction, which is the exact scenario the gate exists for, inverted.

It now runs the comparison the WallstopProto Generator workflow runs: build
each project with AnalyzerPayloadOutputDir redirected at a scratch directory,
then compare those bytes against the committed ones over the UNION of both
sides. Both shipped assemblies are named rather than discovered, so absence is
a failure with a name in it rather than an empty map.

Proved red-green against the real repository, not only the fixture: a single
appended byte reddens it naming the file, --fix restores it byte-identically
and git status is clean afterwards.

Self-test registered in run-contract-tests.js with a red half per guard, each
asserting its own guard's message so a case cannot pass by tripping a
different one. 17 assertions, 0.1 s.

Refs #558, #556

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
#556 asked for a meta-check one level up from "no linter has been left
unreachable": a linter registered in a suite with no self-test that can make
it fail is itself reportable. For a scanner over a corpus that is clean by
construction, a green run is not evidence it still fires.

test-run-repo-lint.js now asserts every scripts/lint-* and scripts/validate-*
has a self-test that INVOKES it and that some registered suite runs. Seven
linters are carried on an explicit allowlist naming the issue that tracks
them (#562); the check refuses an entry that has since gained coverage, so the
list cannot rot in either direction.

lint-changelog.ps1 was the eighth and is fixed rather than allowlisted. It read
CHANGELOG.md from $PSScriptRoot with no way to hand it anything else, which is
why it had no self-test; it now takes -ChangelogPath. Ten red halves, one per
rule that exits non-zero, each asserting its own message.

A stronger-sounding "the self-test names a red half" predicate was written and
DROPPED: all 32 existing self-tests match that vocabulary, so it discriminates
nothing -- it would have been one more check that cannot go red, inside the
test whose subject is checks that cannot go red.

The first draft had #556's own defect. It scanned every file under
scripts/tests/ for a mention of the linter, including this file, which names
linters in allowlists rather than running them: every allowlisted linter read
as covered by the list excusing it. Caught only because the allowlist carries
its own red half. Registries are now excluded by name.

All four branches proved red on the real tree, then green: a de-allowlisted
linter, an allowlist entry that gained coverage, a malformed issue reference,
and the clean run.

Refs #556, #562

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The owner asked on #516 for "a test or function that ensures that we output
the period of each generator (include in our docs) that is ran whenever docs
are refreshed". This is that.

RandomGeneratorMetadataAttribute gains an optional `period:` argument -- named
and defaulted, so no existing annotation or consumer call breaks -- surfaced as
RandomGeneratorMetadata.Period and PeriodLabel. All 20 generators declare one,
and the Random Generators table carries a Period column that
scripts/tests/test-random-periods.js refuses to let drift in either direction:
source without docs, docs without source, or two different values.

Provenance is part of the value rather than a footnote. A period of 2^128
cannot be observed, so where a specification publishes one it is quoted, and
where none exists the value states the MEASURED live state width instead. Four
Excellent ratings in this roster came from repositories that now 404; an
invented period would be that failure one column over.

The measured widths are this session's, taken on 6000.4.6f1 by OR-ing the
state delta over 3,000 NextUint draws per generator: 10 bits for WDoomRandom
(2^10 = the 1024-draw period its notes already claim, independently
corroborated), 251/256 for BlastCircuitRandom, 192/192 for FlurryBurstRandom,
108/160 for IllusionFlow, 28,407 for StormDropRandom.

typecheck green in all four configurations. Verified in the real editor
through RandomGeneratorMetadataRegistry.Snapshot: 20 generators, 0 undeclared,
with the probe gated on a member only the new assembly declares. The gate
proved red both ways -- a perturbed docs cell and a removed declaration -- then
green.

Refs #516, #285

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
PLAN.md gets the session section; .llm/context.md gets three durable rules;
unity-api-costs.md gets the measured relational assignment cost and the
Debug.LogError trap; unity-devcontainer-testing.md gets the GetTypes() sweep
trap.

The relational cost went to unity-api-costs.md rather than the MCP skill
because that skill reached 480 lines -- exactly the CRITICAL threshold -- and
the size gate is a preflight failure, not a warning.

Refs #529, #435, #556, #516

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

verify-shipped-analyzers.js exported PROJECTS with a comment saying it was
kept for the placement contract test. Nothing imports it: test-analyzer-placement.js
enumerates Runtime/Analyzers itself. The export was already dead before this
session; the comment I added invented a consumer for it.

Found by checking my own justification the way the allowlists in
test-run-repo-lint.js require theirs to be checked -- an excuse whose subject
does not exist excuses nothing.

Refs #558

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
fileInvokes() is in the same file, looks like exactly the right helper, and
gives false positives here. Its npm branch expands `npm run <aggregate>`, so
validate:local resolves to 31 scripts and every linter inside one reads as
covered. Measured: five of the seven allowlisted linters came back "invoked"
by test-sync-script-contracts.ps1, which only quotes npm script names in a
string literal whose CONTENTS it is asserting.

That helper answers "does anything run this" -- the assertion above it -- not
"can this go red".

A comment-stripped basename token was also measured, to exclude a linter named
only in prose. It selects exactly the same seven, so it is complexity with no
present effect and was dropped for the same reason the red-half vocabulary
predicate was.

Refs #556

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The first annotation read "~2^61 expected (Romu capacity estimate)". Romu
publishes a CAPACITY -- how much output can safely be drawn -- not a period,
and its generators are non-linear with no guaranteed cycle length. In a column
headed Period a reader takes 2^61 as one, and I had not verified the figure
against the specification either.

It now states the two things that are true: no guaranteed period, and the
128/128 state bits measured live this session.

Refs #516

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings August 25, 2026 06:27

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

The column now carries entries like "unpublished; 251/256 state bits live
(measured)", which is cryptic without the rule behind it. A short section says
that every value is a claim and the suffix says whose -- published
specification, unverified author claim, or a measurement -- and that a
live-state count is a lower bound on state WIDTH rather than a period.

Also names where the value lives (the attribute on the type) and that a
contract test fails the build if the table and the annotation disagree.

Refs #516

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings August 25, 2026 06:31
@wallstop Eli Pinkerton (wallstop) changed the title Make four silent gates able to fail Publish every generator's period Aug 25, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Two small things on the shipped surface.

The attribute declared Period between Quality and Notes, splitting the run of
constructor-ordered properties; the struct next door already declares it after
ReferenceUrl. It now matches both the struct and the constructor.

Its <remarks> named scripts/tests/test-random-periods.js. That path is real but
it is contributor tooling, and this doc shows up in a consumer's IntelliSense.
The remark now explains the rule -- every value says whose claim it is -- and
leaves the enforcement to the contributor docs that already carry it.

typecheck green in all four configurations.

Refs #516

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings August 25, 2026 06:50

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@wallstop

Copy link
Copy Markdown
Collaborator Author

Note for review, correcting one of my own commit messages.

ec7878bc ("Record session 223 findings and plan state") says "PLAN.md gets the session
section; .llm/context.md gets three durable rules; ..."
. The first clause is wrong: PLAN.md is
gitignored (.gitignore:47:/PLAN.md*), so that commit contains only the three .llm/ files, which
is what it should contain. The PLAN.md edits exist locally and never enter the repository.

Nothing durable was lost — the session's findings are in the tracked files (.llm/context.md,
.llm/skills/unity-api-costs.md, .llm/skills/unity-devcontainer-testing.md) and on the issues
(#558, #556, #516, #285, #498, #529, #543, #540, #505, #435, and the Dependabot PR #559). Only the
message overstates what landed, and I would rather flag that than rewrite a pushed commit to hide
it.

Same class as the two claims this PR already corrects in its own code: a dead PROJECTS export
justified by a consumer that does not import it, and RomuDuo's period quoted from Romu's
capacity figure. Three for three on justifications nobody had checked.

The existing note said superseding is "pure waste" and reds the previous
commit. Both true, and both easy to reason around the wrong way: this session
pushed four times, checking each time that the Unity legs were still QUEUED and
concluding nothing was being wasted.

That measured the wrong thing. require-current-pr-head fails every leg of the
superseded run as stale whether or not those legs ever started, so three red
Unity Tests runs are now on this branch and queue depth had nothing to do with
it.

The note also now says what does NOT break, because I nearly wrote the scarier
version: `Unity CI Success` re-resolves the pull request head last and reported
success on all three superseded runs. That is deliberate -- the job carries the
same measurement in its own comment -- so the damage is a misleading run list,
not a blocked gate. Saying otherwise would push the next session into being
over-cautious about a red that does not gate anything.

Held rather than pushed: pushing this would have made it four.

Refs #435

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings August 25, 2026 07:51

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

The standalone leg reddened on ColorQuantizationTests' fuzz test:

    Cutoff 0.8862745 disagreed at channel 226. Expected: False But was: True

Real bug, not a flake. ToThresholdByte returned floor(cutoff * 255f), which is
the ALGEBRAIC inverse of the decode and not the float one -- and this class's
own remarks quote the rule it breaks: never mix the encode and decode steps of
two quantizers. ToNormalized multiplies by the rounded reciprocal 1f/255f;
the threshold multiplied by 255f. Within a ULP of a boundary the multiply by
255 recovers exactly the value the reciprocal lost, so the two disagree.

Measured on 6000.4.6f1 at +/-2 ULP of every channel boundary before the fix:
126 disagreements across 3,820 probes. The fuzz test could only ever find one
of them, and only by luck -- it draws cutoffs from an unseeded PRNG.Instance.

The answer is now defined against ToNormalized itself, which is the comparison
the caller actually makes: seed with the fast floor, then correct by at most
one step in each direction. After: 457,216 probes at +/-3 ULP of every
boundary against every channel, 0 disagreements, saturation unchanged
(NaN 0, -inf 0, +inf 255).

Two deterministic tests replace luck with a walk: every boundary at -1/0/+1
ULP against its neighbouring channels, and the exact pair CI reported.
ColorQuantizationTests 12 pass / 0 fail through the editor, gated on a member
only the fixed assembly declares.

ColorQuantization is not in 3.5.1 (`git ls-tree` on the tag), so this folds
into its Added entry rather than becoming a Fixed one.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@wallstop
Eli Pinkerton (wallstop) merged commit 3825ab7 into main Aug 25, 2026
35 of 36 checks passed
@wallstop
Eli Pinkerton (wallstop) deleted the dev/wallstop/session-223 branch August 25, 2026 15:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ci: verify-shipped-analyzers cannot see a deleted DLL, and is a no-op if its directory moves

2 participants