Conversation
Tests: unit 213/213 + hosted 16/16 green (local xcodebuild)
Tests: unit 213/213 + hosted 16/16 green (local xcodebuild)
3lvis
marked this pull request as ready for review
September 17, 2026 04:52
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
Pinwheel had no linter, no formatter and no pre-merge gate of any kind — Actions has been paused to manual dispatch since GitHub's macOS runners started flaking on the hostless capture tests, so every push since has landed on a promise. This adopts oida, the linter
NetworkingandSwiftSyncalready run, at zero baseline, and puts a gate back on every push by running it where the flake has never reached: Linux.1380 first-party violations at the start, 0 now, with 36 in-place disables that each carry their reason. (Corrected after merge: the body first said 21, which undercounted the directives.)
The approach
bin/lintfetches the pinned release, checksums it against.oida-checksum-<platform>, asserts the version after unzipping, and runs it..oida.ymlcarries the rules, the commit hook callsbin/lint --staged, and.github/workflows/lint.ymlcallsbin/lint --checkonubuntu-latestinswift:6.3.2-noble. It is its own workflow rather than a job inci.yml, becauseci.ymlis the paused one and lint has no reason to wait on it.--fixreformat and its correctionsThe LEARNINGS directory was the graduated pass's leftovers
136 notes, and 120 of them carry the same timestamp — one bulk split of an existing document rather than 136 sessions writing 136 lessons. Reading them against AGENTS.md, that document already carries what they found: "Containment is a UIKit job", "behaviour is a value with no views in it", "Theme is law", "Colours are trait-reactive, fonts are not", "One implementation per component", "A warning only re-emits on a build that recompiles", "Dump the runtime rather than stopping at a search result" — each is a note, already promoted. LEARNINGS.md's own step 2 says to "delete the file once the rule carries everything it said", and that half never ran.
So the notes go, and
git log --diff-filter=D -- LEARNINGS/reads them. Four rules AGENTS.md was missing graduated first: the zero-cooperation capture contract and the hosted-world routing behind it, that an OS bump silently empties SwiftUI's private shape storage, that rows get a tray and a screen gets a sheet, and that a hosting controller reports the size its old content had.Two of them were stale in a way a symptom search would have found: one note says a captured surface must never be a
List, and a later one says a rawListcaptures fully and "this is why the demo stays a rawList".The prose
91 sentences across AGENTS.md, README.md, MIGRATION.md and LEARNINGS.md, 73 in AGENTS.md alone. VOICE.md asks for the positive core rather than the negative foil, and it reads better nearly every time: "never editing source and test in the same step" became "with the test landing in a step of its own ahead of any source edit".
The 21 that stay
Seven are recursive tree walkers with no call site to move into. The rest are seams where the rule's repair would be worse than the violation: the tray parts'
show/detachvocabulary, whose inlining would have a parent reach past a part into what it hosts;PinwheelRecorder's statics, which forward to aprivatesession a caller cannot name; andPinTrayCardPlacement, which owns the card's constraints. Each says so above the disable.UITableViewCell.setDefaultSelectedBackgound()is gone — public, unused, and misspelled at every call site.MIGRATION.mdcarries the three lines that replace it.Testing
Both tiers, local, green at every commit: unit 213/213 and hosted 16/16. The Demo app builds warning-clean against a throwaway derived-data path, which is the only way the warnings gate answers honestly.
Learnings
Three oida issues came out of this, which is the point of adopting it here:
bin/lint --checkreported 0 document violations against this tree where plainoida lintreported 424.--fixrewrites.map(\.value)to a trailing closure inside afor-inheader, which Swift warns about. On a repository that fails CI on any first-party warning, the corrector wrote the build CI rejects.// oida:disable:nextbetween a///and its declaration tripsorphaned_doc_comment; above the doc it disables the wrong line. The shape that works is the reason above the doc and// oida:disable:thistrailing the declaration.Earned its keep: AGENTS.md § Built to be tested — "Almost every bug here has been the same shape: a second copy of the state". Folding
selectedItemlooked safe until the build showed it was passed toPinwheelIndexViewas a closure, and foldingwasPulledtook a test off the seam whose contract it exists to hold. Both got reverted to the seam.