feat: move type generation from atrium to jacquard #94
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR migrates Lexicon-based type generation from Atrium to Jacquard, modernizing the Rust codebase with improved string handling (CowStr), cleaner serialization/deserialization, and explicit borrow semantics. The migration replaces the esquema-cli tool with jacquard-codegen and updates all type references throughout the codebase.
Key Changes:
- Replaced esquema-cli with jacquard-codegen for Rust type generation, including faster installation via cargo-binstall
- Updated all type imports from atrium-api to jacquard-common with new namespace conventions (e.g.,
fm::teal→fm_teal) - Refactored code to use Jacquard's lifetime-aware types with explicit
'staticconversions viaIntoStatictrait
Reviewed Changes
Copilot reviewed 21 out of 22 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/lexicon-cli/src/commands/generate.ts | Switched from esquema-cli to jacquard-codegen with cargo-binstall fallback |
| tools/lexicon-cli/README.md | Updated documentation to reflect jacquard-lexicon tooling |
| services/types/readme.md | Updated installation instructions for jacquard-lexicon |
| services/types/Cargo.toml | Replaced atrium dependencies with jacquard-common and jacquard-derive |
| services/cadet/src/ingestors/teal/feed_play.rs | Migrated to Jacquard types with CowStr handling and lifetime annotations |
| services/cadet/src/ingestors/teal/actor_status.rs | Updated to use Jacquard's Did type and value deserialization |
| services/cadet/src/ingestors/teal/actor_profile.rs | Converted to Jacquard types with blob reference handling changes |
| services/cadet/src/ingestors/car/car_import.rs | Updated CAR import to use Jacquard value deserialization |
| services/cadet/Cargo.toml | Removed atrium-api, added jacquard-common |
| lexicons/fm.teal.alpha/stats/defs.json | Removed required fields from view objects |
| apps/aqua/src/xrpc/*.rs | Added IntoStatic conversions for API responses with lifetime parameters |
| apps/aqua/src/repos/*.rs | Migrated repository layer to Jacquard types with CowStr conversions |
| apps/aqua/Cargo.toml | Replaced atrium-api with jacquard-common |
| README.md | Updated references from esquema to jacquard-lexicon |
| Cargo.toml | Added workspace jacquard dependencies, removed atrium-api |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| let artists = match row.artists { | ||
| Some(value) => { | ||
| from_json_value::<Vec<types::fm_teal::alpha::feed::Artist<'_>>>(value) | ||
| .unwrap_or_default() | ||
| } | ||
| None => vec![], | ||
| }; |
There was a problem hiding this comment.
[nitpick] The lifetime annotation <'_> on Artist is inconsistent with the explicit lifetime used elsewhere in this file. Consider using an explicit lifetime parameter or removing it if the compiler can infer it correctly.
| Did::new(&message.did) | ||
| .map_err(|e| anyhow::anyhow!("Failed to create Did: {}", e))?, |
There was a problem hiding this comment.
[nitpick] The code still uses Did::new() with error handling here, but in actor_status.rs (line 75, 80), the simpler Did::from() is used. Consider using the consistent Did::from() pattern here as well for consistency.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
prob need to cargo update to make sure you got jacquard-lexicon 0.5.3 |
This PR migrates our Lexicon-based type generation from the Atrium ecosystem to Jacquard, resulting in cleaner, more idiomatic Rust code with better memory management.
Jacquard has a bunch of improvements compared to Atrium, including