feat(mex): typed mex operations from the whatspec IR, drop hand-maintained mex_ids - #728
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (9)
💤 Files with no reviewable changes (1)
📝 WalkthroughSummary by CodeRabbit
WalkthroughMigrate MEX request construction to a generic, typed API: add MexRequest and mex_request! macro, pre-serialize MexQuerySpec payloads, replace mex_ids with mex_operations, and update community, newsletter, and groups call sites to use typed operations and payload structs. ChangesMEX API type-safety and macro-based request construction
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Benchmark Results67 unchanged benchmark(s)
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c73d30ad28
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@wacore/src/iq/mex.rs`:
- Around line 162-165: The current use of
serde_json::to_vec(&payload).unwrap_or_default() silently drops serialization
errors into an empty payload; change this to fail fast by using expect() on
serde_json::to_vec(&payload) with a clear panic message referencing the payload
and context (e.g., "failed to serialize mex payload in mex.rs for payload:
{:?}") so payload_bytes is either valid JSON bytes or the process panics loudly;
update the assignment around payload_bytes and any related comments to reflect
the new fail-fast behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: f622ab4b-146c-426c-8ac5-2ac9aa31283c
📒 Files selected for processing (9)
src/features/community.rssrc/features/groups.rssrc/features/mex.rssrc/features/newsletter.rswacore/src/iq/mex.rswacore/src/iq/mex_ids.rswacore/src/iq/mex_operations.rswacore/src/iq/mod.rswacore/src/iq/newsletter.rs
💤 Files with no reviewable changes (1)
- wacore/src/iq/mex_ids.rs
…ained mex_ids Replace the hand-maintained `wacore::iq::mex_ids` doc-id registry with the whatspec-generated typed operations, vendored verbatim as `wacore::iq::mex_operations` (one module per op: `NAME`/`DOC_ID`/`OPERATION_KIND` consts plus typed `Variables`/`Response`). Refresh by re-copying whatspec's `generated/mex/operations.rs`. This fixes a latent bug for free: `get_metadata`/`list_subscribed` were sending persisted-query ids the current WA Web bundle no longer ships (the ops were renamed to `WAWebMexFetchNewsletterJobQuery` / `WAWebMexFetchAllNewslettersMetadataJobQuery`); the vendored file carries the current ids. The response stays parsed by the existing domain parsers over `data: Value` (the generated `Response` mirror is a heuristic structural copy: it lacks enums, types some string fields as numbers, and doesn't flatten/coerce, so it is not a safe deserialize target). The generated `Variables`, in contrast, are a clean typed input. Ergonomics: a `mex_request!` macro pulls `NAME`/`DOC_ID` from a generated module so the op is named once, and `MexRequest<V>` carries the typed variables. They are serialized once in `execute_request` into the wire payload (`MexQuerySpec` holds the bytes), so there is no intermediate `serde_json::Value` tree and a caller-side serialization error surfaces as `MexError::Json` instead of a malformed empty request. Callsites stay `?`-free (the serialization `?` lives in `execute_request`). `update_group_property` is the one op the generated mirror types too loosely (its `update` is a one-of object the heuristic flattened to `String`); it now uses a hand-typed `GroupPropertyUpdate` enum (`#[serde(rename_all = "snake_case")]`) so that path is typed too, with a test locking the wire shape. No `serde_json::Value` remains on the mex request path. Verified: build, clippy --all-targets -D warnings, and the full test suite (excluding e2e) all pass. The serialized request payloads are wire-equivalent to the previous `json!` ones (None fields are skipped).
c73d30a to
9ae4f36
Compare
Replaces the hand-maintained
wacore::iq::mex_idsdoc-id registry with the whatspec-generated typed operations, vendored verbatim aswacore::iq::mex_operations. One module per op, each withNAME/DOC_ID/OPERATION_KINDconsts plus typedVariables/Response. Refresh by re-copying whatspec'sgenerated/mex/operations.rs.Fixes a latent bug for free
get_metadataandlist_subscribedwere sending persisted-query ids the current WA Web bundle no longer ships. The ops were renamed upstream (toWAWebMexFetchNewsletterJobQueryandWAWebMexFetchAllNewslettersMetadataJobQuery); the vendored file carries the current ids, so these stop silently failing.Typed input, domain-parsed output
The generated
Variablesare a clean typed input and replace the previousjson!objects. The response stays parsed by the existing domain parsers overdata: Value: the generatedResponsemirror is a heuristic structural copy (no enums, some string fields typed as numbers, no flatten/coerce), so it is deliberately not used as a deserialize target.NewsletterMetadataand friends stay the domain layer.Ergonomics + fewer allocations
A
mex_request!macro pullsNAME/DOC_IDfrom a generated module, so the op is named once:MexRequest<V>/MexQuerySpec<V>now hold the typed variables and serialize them straight to the wire inbuild_iq, dropping the intermediateserde_json::Valuetree. Construction is infallible, so callsites lose the?.The one loosely-typed op
update_group_property'supdateis a one-of object the generator's heuristic flattened toString. It now uses a hand-typedGroupPropertyUpdateenum (#[serde(rename_all = "snake_case")]) so that path is typed too, with a unit test locking the wire shape. Noserde_json::Valueremains on the mex request path.Verification
cargo build,cargo clippy --all-targets -- -D warnings, and the full test suite (excluding e2e) all pass. The serialized request payloads are wire-equivalent to the previousjson!ones (skip_serializing_if = "Option::is_none"drops the unset fields). This supersedes the closed #727 (build-time generation crate) in favor of vendoring the typed operations directly.