fix(codegen): Two remaining gaps blocking a clean regeneration - #345
Merged
Conversation
…omments `app.bsky.ageassurance.defs` carries `"description": ""`, which generated a bare `///` and tripped `clippy::empty_docs`. Since CI runs with `RUSTFLAGS: "-D warnings"`, that alone fails the build on regeneration. Treat an empty description the same as a missing one, both for the per-item `#[doc]` and the schema-level `#![doc]`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`generate_modules` rebuilt the NSID from the snake_case'd file stem, so a record whose NSID leaf is camelCase never matched its own schema and got no `Collection` impl. Every record so far had a single-word leaf (`profile`, `status`, `listitem`, ...), so this went unnoticed until `app.bsky.actor.contentVisibilityDeclaration`. The mismatch is not cosmetic: `KnownRecord` is built from schema ids and does gain the variant, so downstream crates get a `KnownRecord` arm they cannot satisfy because the `Collection` type does not exist. Match on the snake_case'd NSID leaf instead, and use the schema's own id for the `NSID` constant. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This was referenced Aug 18, 2026
knasher
added a commit
to knasher/atrium
that referenced
this pull request
Aug 20, 2026
`cargo build` does not compile the test target, so nothing catches codegen dropping a `Collection` impl. atrium-rs#345 fixed exactly that: `app.bsky.actor. contentVisibilityDeclaration` is the first record whose NSID leaf is camelCase, and the NSID was rebuilt from the snake_case'd file stem, so the record never matched its own schema and got no `Collection` impl. `KnownRecord` is keyed on schema ids, so it *did* gain the variant, leaving downstream crates with an arm they could not satisfy. Two assertions, neither coupled to lexicon contents: - Naming `ContentVisibilityDeclaration::NSID` requires the `Collection` impl to exist; comparing it against the literal requires it to carry the NSID verbatim rather than a mangled form. - A never-called function taking `<ContentVisibilityDeclaration as Collection>::Record` and returning the corresponding `KnownRecord` variant asserts at compile time that the two halves agree on the record type. Drifting the associated type fails to build here while the NSID assertions still pass, so this catches a case the runtime checks cannot. Verified by reintroducing the atrium-rs#345 breakage against the current tree: deleting the `Collection` impl fails the test target to compile, as does pointing its `Record` at a different record type. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
Summary
Follow-up to #342. That PR got
lexgenparsing the current lexicons, but regeneratingatrium-apiagainst them still doesn't come out CI-clean. Two codegen gaps remain — both are pre-existing bugs newly triggered by lexicons that landed upstream, and both must be fixed before a regeneration can go green.Changes
Skip empty descriptions.
app.bsky.ageassurance.defscarries"description": "", which generated a bare///and trippedclippy::empty_docs. CI runs withRUSTFLAGS: "-D warnings", so this alone fails the build. Empty descriptions are now treated the same as missing ones, for both the per-item#[doc]and the schema-level#![doc].Generate
Collectionimpls for camelCase record NSIDs.generate_modulesrebuilt the NSID from the snake_case'd file stem, so a record whose NSID leaf is camelCase never matched its own schema and got noCollectionimpl. Every record so far had a single-word leaf (profile,status,listitem, …), so this went unnoticed untilapp.bsky.actor.contentVisibilityDeclaration.This one is not cosmetic:
KnownRecordis built from schema ids and does gain the variant, sobsky-sdk'screate_recordmatch gets an arm it cannot satisfy —atrium_api::app::bsky::actor::ContentVisibilityDeclarationsimply doesn't exist. The whole workspace fails to build.Verification
Generated against
bluesky-social/atproto@5c3b7c9c8(402 lexicons) with the codegen before and after this PR. The output diff is exactly 11 lines — the empty///disappearing and the newCollectionimpl appearing — and nothing else moved:All 20 in-namespace records get a
Collectionimpl (was 19).With these fixes plus the downstream catch-up,
cargo check --workspace --all-features --all-targetsandcargo test --workspace --all-featuresboth pass on a full regeneration. That regeneration will follow as a separate PR.🤖 Generated with Claude Code