feat(business): add catalog, product and business profile operations - #1252
Conversation
Adds the read side of the business catalog over MEX and the write side of the business profile over IQ, matching how WhatsApp Web splits the two. Catalog, collections and order lookup are MEX persisted queries (QueryCatalog, QueryProductCollections, BizQueryOrder). WhatsApp Web has no IQ fallback for them: a failed xwa_product_catalog_get_product_catalog raises CatalogUnknownError rather than retrying over w:biz:catalog, and the only catalog IQ left in the bundle is product_list plus signed_user_info. Business profile writes stay on IQ w:biz as a business_profile node with v="3" and mutation_type="delta", covering address, coordinates, description, email, websites, categories and opening hours, plus setting and removing the cover photo. Two shapes the generated mex Variables mirrors get wrong, so the catalog and collection queries build their variables by hand: WhatsApp Web sends limit, width, height, collection_limit and item_limit as strings, and allow_shop_source as ALLOWSHOPSOURCE_TRUE/_FALSE rather than a boolean. The order query does send numeric dimensions and uses the generated type. Money is kept as an i64 in thousandths of the currency unit, matching priceAmount1000 in the protobuf and formatAmount1000 in the client, so a price is never routed through a float. Absent fields are Option rather than a blank or zero, including the booleans, which arrive as strings (is_hidden is the enum spelling ISHIDDEN_TRUE, not a JSON boolean). Product create/edit/delete is deliberately absent: those IQs are not in the WhatsApp Web bundle at all, and the cover-photo upload leg is not covered either, since biz-cover-photo returns fbid/meta_hmac/ts instead of the url/direct_path pair the upload pipeline parses. set_cover_photo therefore takes an upload receipt rather than image bytes.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI (base), Organization UI (inherited) Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe PR adds typed business catalog, collection, order, profile, and cover-photo APIs. It adds parsing, validation, paging, pricing, tests, and public exports. It also refreshes generated MEX operations and input schemas. ChangesBusiness API
MEX operation schema refresh
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant Business
participant MEXOperations
participant IQTransport
participant Parser
Client->>Business: request catalog, collection, or order
Business->>MEXOperations: send typed MEX query
MEXOperations-->>Business: return response
Business->>Parser: validate and parse response
Parser-->>Client: return typed result
Client->>Business: update profile or cover photo
Business->>IQTransport: send business IQ mutation
IQTransport-->>Client: return mutation result
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
|
| Filename | Overview |
|---|---|
| src/features/business.rs | Adds the public business feature API and parsers; collection pagination now preserves paging.after and serializes it on the next request. |
| wacore/src/iq/business.rs | Adds typed IQ specifications and validation for business-profile and cover-photo mutations. |
| wacore/src/iq/mex_operations.rs | Refreshes generated persisted-query definitions used by the new MEX-backed operations. |
| src/features/mod.rs | Registers the business feature and re-exports its public types. |
| src/lib.rs | Exposes the new business API types from the crate root. |
Sequence Diagram
sequenceDiagram
participant App
participant Business
participant MEX
participant WhatsApp
App->>Business: get_collections(options)
Business->>MEX: collections request with options.after
MEX->>WhatsApp: persisted query
WhatsApp-->>MEX: collections + paging.after
MEX-->>Business: response data
Business-->>App: "Collections { collections, after_cursor }"
App->>Business: get_collections(after_cursor)
Reviews (15): Last reviewed commit: "fix(business): keep the shimmed product ..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 4 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
…dation Addresses review findings on the catalog slice. get_collections sent CollectionOptions::after but returned a bare Vec<Collection>, discarding the paging cursor the server replies with, so a business with more collections than collection_limit looked complete and could not be paged past the first response. WhatsApp Web reads paging.after here, so the response now returns a Collections page carrying the cursor. Unlike the catalog this paging object has no "before", and the type says so. parse_collections also accepted two malformed shapes: a non-object entry indexed to Null on every field and became a plausible nameless empty collection, and a "products" that was present but not an array fell through to empty rather than erroring the way the catalog path does. Both are now typed errors. An absent "products" key is still a legitimately empty collection. Profile coordinates were formatted straight to wire text, so NaN and the infinities would have been sent as "NaN"/"inf" and an out-of-range latitude as a point that does not exist. The server rejects the whole delta in that case, silently losing the other fields in the same update, so coordinates are now range-checked before any node is built. Finally, the profile-write types are the arguments and error of this feature's public methods but lived only in wacore, forcing callers to reach into wacore::iq::business to construct an update. They are now re-exported through features::business and the crate root, matching the convention the other features follow.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
|
All five review findings were valid — fixed in 563856c. Collections cursor (Greptile + cubic). Real bug, and the worst of the five: Non-object collection entry / non-array Coordinate validation. Re-exports. Correct per the convention in Generated by Claude Code |
There was a problem hiding this comment.
0 issues found across 4 files (changes from recent commits).
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Requires human review: Adds new public business APIs with protocol, currency, and delta-write tradeoffs (MEX-vs-IQ choice, amount_1000 scaling, profile mutation semantics); diffs are truncated, so the claimed ground truth and behaviors are not fully verifiable without human review.
Re-trigger cubic
📦 Binary size report
.text per crate
Baseline: |
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 563856c80e
ℹ️ 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".
| let response = self | ||
| .client | ||
| .mex() | ||
| .query(mex_request!(query_catalog, catalog_variables(jid, options))) |
There was a problem hiding this comment.
This newly exposed path obtains its persisted-query ID from query_catalog::DOC_ID, which is still 9916553288394782, even though the WhatsApp Web ground-truth check documented for this change found the current ID to be 30445081048424116. When the server no longer accepts the prior persisted document, every get_catalog call fails before response parsing, so refresh the vendored MEX operations before wiring this query.
AGENTS.md reference: AGENTS.md:L5-L5
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Valid, and I had the severity wrong — fixed in cdac3ac.
I'd disclosed the drift in the PR body but filed it as a separate chore on the assumption that every MEX consumer shared the exposure. That assumption was wrong, and checking it is what changed my mind: of the 11 operations this repo actually calls, query_catalog is the only one whose id had rotated — the other ten are current (repo-wide, 4 of 126 resolvable ops drifted). So the stale id sits on exactly the query this PR introduces, and no existing caller was ever exercising it. That makes it this PR's defect, not a background chore.
Fixed by refreshing the whole vendored file rather than editing the one constant: the header forbids hand-edits, and a mixed-vintage file would misreport which WhatsApp build it describes. Regenerated with whatspec at WA 2.3000.1044659339 from the pinned 497-bundle set.
Worth recording how that was verified, since a regeneration is only as trustworthy as its inputs. My first attempt ran against 494 bundles and whatspec refused to write — its integrity gate caught that appstate had extracted 0 actions. Three bundles had failed to download because their filenames exceed 255 bytes. With the full 497, the run succeeds and the regenerated mex IR matches the committed whatspec snapshot byte for byte, so the output reflects that snapshot rather than an incidental fetch.
Blast radius, checked rather than assumed: one in-use id changes (query_catalog), ten unchanged, fetch_native_ads_mvp_eligibility drops with zero references anywhere in the tree, nine new ops arrive with no callers. Build, 1386 + 1465 lib tests, doctests and clippy --all-features all clean.
Generated by Claude Code
| open_time: Some(open_time), | ||
| close_time: Some(close_time), |
There was a problem hiding this comment.
When a caller supplies a value outside a local day, such as u32::MAX, with_hours accepts it and the mutation builder serializes it verbatim as open_time or close_time. The server can then reject the entire profile delta, including unrelated fields in the same update; validate the verified minute bounds in the constructor or update specification rather than allowing every u32.
AGENTS.md reference: AGENTS.md:L53-L56
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Valid — fixed in cdac3ac. It's the same failure mode as the coordinate check, and leaving one validated while the other took any u32 was an inconsistency in my own design.
I went to the bundle for the bound rather than assuming, because the unit determines it. WAWebSmbUtilsTimeUtils settles it: minutesToTime is moment.utc().startOf("day").add(n, "minutes"), and its inverse timeStringToMinutes parses a within-day format and returns duration(diff).asMinutes(). So the domain is minutes past local midnight, and 1439 (23:59) is the largest value WhatsApp Web can produce — hence value < 1440.
Validation went in the spec constructor rather than with_hours, matching where check_coordinate runs — before any node is built, so a bad value can't take an unrelated field in the same delta down with it, and with_hours stays infallible.
One thing I deliberately did not add, despite finding a precedent for it. WA Web's hours editor has an i>l branch that coerces one endpoint when open > close, so its picker never emits an inverted range. That is a UI affordance, not a wire constraint — a range crossing midnight (20:00–02:00 is the obvious real case) is legitimate, and other clients may well send one. Enforcing ordering here would reject valid data, so the check bounds each endpoint independently and (1200, 120) is covered by a test asserting it stays accepted.
Generated by Claude Code
…r times
Addresses the Codex review on the catalog slice.
query_catalog's persisted-document id had rotated upstream
(9916553288394782 -> 30445081048424116), so get_catalog — added in this
branch — was being wired to an id WhatsApp Web no longer sends. Of the
eleven mex operations this repo actually calls, it is the only one that
had drifted, so the new code path was the single exposed caller rather
than one of many.
mex_operations.rs is vendored verbatim from whatspec's generated
operations.rs, so this refreshes the whole file rather than editing the
id in place: the header forbids hand-edits, and a mixed-vintage file
would misreport which WhatsApp build it describes. Regenerated with
whatspec at WA 2.3000.1044659339 from the pinned 497-bundle set, whose
mex IR reproduces the committed whatspec snapshot byte for byte — the
refresh is faithful to that snapshot, not to an incidental fetch.
The refresh moves one other in-use id not at all (the remaining ten are
unchanged), drops fetch_native_ads_mvp_eligibility, which nothing
references, and adds nine operations with no callers yet.
Separately, open_time and close_time accepted any u32 and were formatted
straight to wire text, so a value past the end of the day would be sent
verbatim and the server would reject the entire profile delta, silently
losing the unrelated fields in the same update — the failure mode the
coordinate check already guards against. They are minutes past local
midnight: WhatsApp Web renders them as startOf("day").add(n, "minutes")
and only ever derives them by parsing a within-day time string, so 1439
is the largest value it emits. Ranges that cross midnight stay legal;
WA Web's editor coerces open > close in its own picker, but that is a UI
affordance rather than a wire constraint that other clients share.
Dismissed because a newer commit was pushed; Greptile will re-review the current head.
There was a problem hiding this comment.
All reported issues were addressed across 2 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
The business-hour variant was inserted between InvalidCoordinate's doc comment and InvalidCoordinate itself, so both blocks stacked on the new variant and the coordinate error documented nothing. Each variant now carries its own rationale.
Dismissed because a newer commit was pushed; Greptile will re-review the current head.
There was a problem hiding this comment.
0 issues found across 2 files (changes from recent commits).
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Requires human review: Auto-approval blocked by 1 unresolved issue from previous reviews.
Re-trigger cubic
Both sides added re-exports to the same sorted list in src/lib.rs: the business catalog types here, and NewsletterAdminInfo, NewsletterAdminProfile and NewsletterFollower upstream. Resolved as the union of the two, so neither side loses an export.
Dismissed because a newer commit was pushed; Greptile will re-review the current head.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0d062a7a2b
ℹ️ 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".
| OrderProduct { | ||
| id: opt_str(&product["id"]), | ||
| name: opt_str(&product["name"]), | ||
| price: parse_price(&product["price"], currency.as_deref()), | ||
| quantity: opt_i64(&product["quantity"]), |
There was a problem hiding this comment.
For orders containing product variants such as size or color, the persisted response exposes the selection through biz_query_order::Products.variant_info.variant_properties, but this conversion drops it entirely. Callers fulfilling such orders receive the base product, quantity, and price without knowing which variant was ordered, so expose the variant properties on OrderProduct and parse them here.
AGENTS.md reference: AGENTS.md:L5-L5
Useful? React with 👍 / 👎.
| .as_array() | ||
| .ok_or_else(|| BusinessError::malformed(OP, "order.products is not an array"))? | ||
| .iter() | ||
| .map(|product| { |
There was a problem hiding this comment.
When order.products contains a malformed non-object entry, indexing that value returns Null for every field and this map produces a plausible OrderProduct with every optional field absent. This silently corrupts the order result instead of returning MalformedResponse, unlike the catalog and collections parsers, so check product.is_object() before constructing the line item.
Useful? React with 👍 / 👎.
| pub fn with_hours( | ||
| day_of_week: DayOfWeek, | ||
| mode: BusinessHourMode, | ||
| open_time: u32, | ||
| close_time: u32, |
There was a problem hiding this comment.
Because both constructors accept any BusinessHourMode, callers can create new(day, SpecificHours), which emits no required range, or with_hours(day, Open24H, ...), which emits times for a mode documented as carrying none. These are stanza shapes WhatsApp Web does not produce and can cause the server to reject the entire profile delta; specialize the constructors or validate the mode/time combination in BusinessProfileUpdateSpec.
AGENTS.md reference: AGENTS.md:L5-L5
Useful? React with 👍 / 👎.
| pub use wacore::iq::business::{ | ||
| BUSINESS_PROFILE_MAX_WEBSITES, BusinessHourMode, BusinessHoursConfig, BusinessHoursUpdate, | ||
| BusinessProfile, BusinessProfileUpdate, BusinessProfileUpdateError, CoverPhotoUpload, | ||
| DayOfWeek, |
There was a problem hiding this comment.
This re-export includes BusinessProfile but omits BusinessHours and BusinessCategory, even though they are the public types of its business_hours and categories fields. A whatsapp-rust caller that needs to name those result types in a signature must therefore add and import wacore, defeating the feature-level public surface this block is intended to provide; re-export both here and from the crate root.
AGENTS.md reference: AGENTS.md:L43-L43
Useful? React with 👍 / 👎.
| Ok(Collection { | ||
| id: opt_str(&collection["id"]), | ||
| name: opt_str(&collection["name"]), | ||
| products, | ||
| review_status: opt_str(&collection["status_info"]["status"]), |
There was a problem hiding this comment.
For rejected or appealable collections, the persisted response's StatusInfo2 contains can_appeal, reject_reason, and commerce_url, but this conversion retains only status. A merchant therefore cannot learn why a collection was rejected or where and whether to appeal from get_collections; add these fields to Collection and parse the complete moderation status.
AGENTS.md reference: AGENTS.md:L5-L5
Useful? React with 👍 / 👎.
…d mode pairing Addresses the second Codex review. All five findings check out against the whatspec IR at WA 2.3000.1044659339. Order line items dropped variant_info.variant_properties, which the persisted response does carry. Every variant of one listing shares an id, name and price, so without the chosen size or colour an order for a variant product cannot be fulfilled at all. OrderProduct now exposes them. Collections kept only status_info.status and discarded can_appeal, reject_reason and commerce_url. A rejection is only actionable with the reason and the appeal route, and unlike the product status_info — which really does carry just status and can_appeal — the collection object has all four. parse_order also indexed line items without checking their shape, so a non-object entry read back as a plausible product with every optional field absent, rather than erroring the way the catalog and collection parsers do. Business hours accepted any mode/time combination. WhatsApp Web emits open_time and close_time exactly when a day has a range and reads them back only for specific_hours, so a ranged open_24h — or a specific_hours day with no range — is a stanza it never produces and the server can reject the whole delta over. An unrecognised mode is deliberately left alone: BusinessHourMode::Other exists so a mode added later still round trips, and guessing its arity would turn that into a hard failure. Finally, BusinessProfile's business_hours and categories are typed BusinessHours and BusinessCategory, neither of which was re-exported, so naming a result type in a signature still meant depending on wacore. Both now come through the feature surface and the crate root.
Dismissed because a newer commit was pushed; Greptile will re-review the current head.
|
All five findings from the second Codex pass check out against the whatspec IR at WA Order variant properties. Collection moderation status. Also real, and the asymmetry is the interesting part. A collection's Non-object order line item. Same class as the collections case fixed earlier; Mode/time pairing. Confirmed in the bundle from both directions — the writer emits times iff a day has a range ( One deliberate limit: an unrecognised mode is left alone rather than validated. Nested profile re-exports. Correct — Verified: 1394 + 1492 lib tests (6 new), doctests, Generated by Claude Code |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@src/features/business.rs`:
- Around line 1491-1524: Update the malformed-shape assertions in
malformed_nested_objects_are_errors to include distinct failure messages
identifying each catalog, collection, and order contract, including paging,
media, status_info, compliance_info, nested images, and variant_info cases.
Preserve the existing inputs and expected is_err/is_ok behavior while ensuring a
failed assertion names the malformed field or parser path.
- Around line 737-753: Change parse_importer_address to return
Option<ImporterAddress> directly, preserving its existing None-for-empty and
Some-for-populated behavior. Update its call site to remove the ? operator when
obtaining this value, while leaving object() responsible for error handling.
🪄 Autofix
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 (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 5d0f8901-b008-4ca0-a1e8-0582689ada4c
📒 Files selected for processing (3)
src/features/business.rssrc/features/mod.rssrc/lib.rs
There was a problem hiding this comment.
All reported issues were addressed across 1 file (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
Both raised independently by CodeRabbit and cubic on the previous commit. parse_importer_address kept a Result after the object guard moved into `object()`, so every path returned Ok and the `?` at the call site signalled a failure mode that no longer existed. It returns Option now. The malformed-shape test packed nine contracts into bare is_err() assertions, so a regression reported a line number and nothing else. The catalog and order cases are table-driven with the field named in the assertion message, and the importer_address shape — previously only covered indirectly — gets its own case.
Dismissed because a newer commit was pushed; Greptile will re-review the current head.
There was a problem hiding this comment.
0 issues found across 1 file (changes from recent commits).
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Requires human review: Auto-approval blocked by 1 unresolved issue from previous reviews.
Re-trigger cubic
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a88b82750f
ℹ️ 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".
| let sale_price = parse_price(&sale["price"], currency.as_deref()).map(|price| SalePrice { | ||
| price, | ||
| start_date: opt_str(&sale["start_date"]), | ||
| end_date: opt_str(&sale["end_date"]), |
There was a problem hiding this comment.
When a response contains only start_date or only end_date, this parser exposes a half-populated window even though SalePrice documents that WhatsApp Web considers the window set only when both endpoints are present. Consumers can consequently schedule or display a promotion using a period the official client treats as unset; normalize both fields to None unless both parse successfully, or reject the partial shape as malformed.
AGENTS.md reference: AGENTS.md:L5-L5
Useful? React with 👍 / 👎.
SalePrice documented that WhatsApp Web treats the window as set only when
both dates are present, but the parser surfaced each date independently,
so a response carrying one of them produced a half window the doc said
could not exist. The doc was right and the code was not.
The bundle settles it in both transports: the MEX reader is
`start_date != null && end_date != null ? {sale_start_date, sale_end_date}
: null`, and the IQ reader gates the same pair on
`hasChild("start_date") && hasChild("end_date")`. Either way a lone
endpoint is discarded rather than carried, so normalising it away here
mirrors the client instead of inventing a period it would not honour.
The sale price itself is unaffected — only the window is dropped.
Dismissed because a newer commit was pushed; Greptile will re-review the current head.
|
Re: Codex's "Pair sale window endpoints" on This one was a straight contradiction between my own doc and my own code. I went back to the bundle rather than trusting either side of that contradiction, and it settles it identically in both transports:
The doc was right; the code was wrong. A lone endpoint is discarded by the official client, which makes normalising it away here mirroring rather than inventing — the same distinction I've applied throughout on what to keep versus what to drop, and the reason this one goes the opposite way from the importer and variant findings, where the server really did send data I was throwing out. Scope worth being precise about: only the window is dropped, never the price. A response with a sale price and one date still yields the discounted price; it just doesn't claim a period. The test covers both directions and asserts the both-present case survives untouched. Verified: 1395 + 1497 lib tests, Generated by Claude Code |
There was a problem hiding this comment.
0 issues found across 1 file (changes from recent commits).
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Requires human review: Auto-approval blocked by 1 unresolved issue from previous reviews.
Re-trigger cubic
Same additive collision as the previous merge, now with the bot-list and new-chat-capping work upstream: both sides register a module in src/features/mod.rs and append to the sorted re-export lists there and in src/lib.rs. Resolved as the union throughout, so neither the business surface nor upstream's Bot* and NewChatMessageCapping exports are lost.
Dismissed because a newer commit was pushed; Greptile will re-review the current head.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9ff9126247
ℹ️ 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".
| retailer_id: opt_str(&value["retailer_id"]), | ||
| name: opt_str(&value["name"]), | ||
| description: opt_str(&value["description"]), | ||
| url: opt_str(&value["url"]), |
There was a problem hiding this comment.
When a catalog or collection product supplies only shimmed_url, this conversion exposes Product::url as None, even though both vendored response schemas include shimmed_url separately from url (wacore/src/iq/mex_operations.rs:7153-7158 and 7741-7747). Preserve that field or use it as the fallback so callers do not lose the server-provided product link.
AGENTS.md reference: AGENTS.md:L5-L5
Useful? React with 👍 / 👎.
| /// Products returned inline per collection. | ||
| pub item_limit: u32, |
There was a problem hiding this comment.
When a collection contains more products than item_limit (51 by default), get_collections returns only that inline prefix and exposes no cursor or follow-up operation for retrieving the remainder, so callers can silently treat a truncated collection as complete. The vendored query_product_single_collection operation accepts an after cursor and returns its own paging cursor (wacore/src/iq/mex_operations.rs:8066-8087 and 8338-8355); expose that paginated path or otherwise make truncation observable.
AGENTS.md reference: AGENTS.md:L5-L5
Useful? React with 👍 / 👎.
…cation Two findings on the merged branch. shimmed_url is sent alongside url rather than instead of it, arrives with no opt-in, and was being dropped — so a product whose link the server shimmed lost that form entirely. Both are surfaced as received. Using one as a fallback for the other was tempting but would bake in a preference the wire does not express: the shim adds interstitial handling, and which to open is the caller's policy call. The collections query returns only the first item_limit products of each collection and carries no per-collection cursor, so a large collection comes back truncated with nothing in the response marking it. That is the same shape of trap as the collections cursor fixed earlier — a list that looks complete and is not. Reading one to the end needs a different operation, WAWebQueryProductSingleCollectionQuery, which is vendored with its own after cursor but not wired up here; until it is, the length of the list against item_limit is the only truncation signal, and the type now says so rather than leaving callers to discover it.
Dismissed because a newer commit was pushed; Greptile will re-review the current head.
|
Both Codex findings on the merge check out — handled differently, and the difference is the point.
I did not take the "use it as a fallback for Collection product truncation — documented, not implemented. The finding is correct and the trap is real: But the remedy isn't a parser fix — it's a different operation. What I could close cheaply, I did: Verified: 1413 + 1511 lib tests (33 in the business feature), Generated by Claude Code |
There was a problem hiding this comment.
0 issues found across 1 file (changes from recent commits).
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Requires human review: This PR adds a new public business API surface (catalog, collections, order, and profile mutations) and refreshes vendored MEX operations. The protocol choices, response-shape contracts, and mutating profile behavior are product/API decisions a human should sign off.
Re-trigger cubic
|
Correction on
That variant came from Two things follow that are worth knowing before the version decision:
I've deliberately left the upstream one alone — fixing another PR's break inside a feature branch isn't mine to do, and silently adding Generated by Claude Code |
Third merge with the same conflict in the same place: the
`pub use features::{…}` list in `src/lib.rs`. rustfmt re-wraps the whole
block, so any two additions collide regardless of where they land. This
branch adds `AppStateSettings` and `QuickReplies`; main adds the business
profile and catalog surface from #1252. Took the union.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Up7G7AwUp1q8KqaGCAugJy
2 participants
Summary
Business existed here only for reading a profile and parsing notifications. This adds:
get_catalog— one page of a business's products, with cursor paging.get_collections— one page of product collections, each with its items inline.get_order— an order's line items and totals, from the id + token an order message carries.update_profile— delta mutation of address, latitude/longitude, description, email, websites, categories and opening hours.set_cover_photo/remove_cover_photo.Left out of this slice: product create/edit/delete, and the image-upload leg of the cover photo. Both are explained under Checked and not changed and Media — neither is a shortcut, both are blocked on ground truth rather than on effort.
Ground truth
The MEX-vs-IQ question resolves per operation, not globally. Evidence is WA Web
2.3000.1044659339— whatspec IR from a fresh clone, plus the full 497-file pinned bundle set fetched frombundles.lock.jsonand grepped directly.WAWebQueryCatalogQueryWAWebQueryProductCollectionsQueryWAWebBizQueryOrderJobQueryw:biz→business_profile v="3" mutation_type="delta"Why MEX for catalog/collections/order.
w:biz:catalogstill exists in the bundle, but the IR extracts only two stanzas under it —QueryGetSignedUserInfoandQueryProductListCatalog(a by-id status query). Baileys'product_catalog,collections,product_catalog_add/edit/deleteare not there. Decisively, the catalog query has no IQ fallback:xwa_product_catalog_get_product_catalogfailing raisesCatalogUnknownError. The one operation that does fall back isget_product_list("xwa_product_catalog_get_product_list failed, IQ fallback"), and its A/B propgraphql_get_product_list = false— so the survivingw:biz:catalogIQ is exactly the one MEX op still gated off. The two transports are the same server schema: Baileys' IQ bodies match the MEXvariablesShapefield-for-field (jid/limit/width/height/after;biz_jid/collection_limit/item_limit). Baileys implements the legacy smax-IQ transport of a schema WA Web has since moved to MEX.Why IQ for the profile. Here Baileys is right and the whatspec IQ extractor is wrong —
w:biz/business_profileis built throughWAWap.wap(...)rather than the standard builder, so it lands in the IR's"iq builder substring present but no AST iq call": 5drop bucket. The bundle has it plainly, including fields Baileys lacks (latitude,longitude,categories,price_tier,service_areas) and a detail it gets wrong: WA Web emits at most two<website/>nodes, and a lone empty<website/>to clear the list.BUSINESS_PROFILE_MAX_WEBSITESand a typed error encode that.Correction to the pre-research in the task.
graph_ql_product_catalog_get_public_key,report_productandbiz_create_orderare real, and out of scope as stated.Vendored MEX operations refreshed
The task's pre-research noted
mex_operations.rswas stale (WA2.3000.1042742319). I originally filed that as a separate chore; that was wrong, and the check that corrected it is worth stating: of the 11 MEX operations this repo actually calls,query_catalogis the only one whose persisted-document id had rotated (9916553288394782→30445081048424116). The other ten are current. So the one stale id sat on precisely the query this PR introduces — a defect in this change, not background drift.mex_operations.rsis vendored verbatim from whatspec's generatedoperations.rs, so this refreshes the whole file rather than editing the constant: the header forbids hand-edits, and a mixed-vintage file would misreport which WhatsApp build it describes.The regeneration is verified, not assumed. A first run against 494 bundles was refused by whatspec's own integrity gate — appstate extracted 0 actions, because three bundles had silently failed to download (their filenames exceed the 255-byte limit). With the complete 497, the run succeeds and the regenerated mex IR matches the committed whatspec snapshot byte for byte, so the output reflects that snapshot rather than an incidental fetch. Blast radius: one in-use id changes, ten unchanged,
fetch_native_ads_mvp_eligibilitydrops with zero references in the tree, nine new ops arrive with no callers.Design
Money is the part worth reviewing. WhatsApp scales money by 1000, not 100 —
priceAmount1000is anint64in the protobuf, and the client formats throughformatAmount1000/u/1e3. The GraphQL reader ish(e) = typeof e === "string" ? (e.length > 0 ? parseInt(e,10) : null) : e, which pins three things: the scalar is a decimal string, a bare number is tolerated in the same position, and empty string means absent, not zero.Priceis therefore{ amount_1000: i64, currency: Option<String> }, never a float, and dividing for display is the caller's decision.Optional means
Option, including booleans. OnlyProduct::idis guaranteed — WA Web asserts on it, so a product without one is a typed error rather than a skipped row. Everything else isOption, because afalsefor an absent flag is as much a sentinel as an empty string. This matters concretely: the IR mirrorsis_hidden,is_sanctioned,belongs_toandcan_appealas booleans, and all four are strings on the wire —is_hiddenuses its own spellingISHIDDEN_TRUE, the rest are"true"/"false". The parser accepts both spellings and leaves anything unrecognisedNone.ProductAvailabilityis aWireEnumover the GraphQL names (IN_STOCK,OUT_OF_STOCK,AVAILABLE_FOR_ANOTHER_POSTCODE) with a fallback variant, so a new value degrades one field instead of failing the page.Paging is asymmetric between the two queries, and the types say so rather than smoothing it over: the catalog returns
afterandbefore, the collections response carriesafteronly.A second IR mirror correction, this one affecting the request. WA Web stringifies
limit,width,height,collection_limitanditem_limit, and sendsallow_shop_sourceasALLOWSHOPSOURCE_TRUE/_FALSE. The generatedVariablestype them asi64/bool. The catalog and collection queries build their variables by hand for that reason; the order query, whoseimage_dimensionsgenuinely are numbers, uses the generated type. Tests assertis_string()/is_number()so this cannot silently regress.Malformed responses fail rather than degrade. A non-object collection entry, or a
productsfield present but not an array, are typed errors — either would otherwise read back as a plausible empty collection. An absentproductskey is still a legitimately empty collection.Values that would poison a delta are rejected before any node is built. Two cases, one failure mode: the server rejects the whole
business_profiledelta on a bad field, silently losing the unrelated fields in the same update. Coordinates are range-checked becausef64::to_stringwould otherwise renderNaN/infverbatim. Opening times are bounded to the day:WAWebSmbUtilsTimeUtilsdefinesminutesToTimeasstartOf("day").add(n, "minutes")and its inverse parses a within-day format, so the domain is minutes past local midnight and 1439 is the largest value WA Web emits. Ranges crossing midnight stay legal — WA Web's editor coercesopen > closein its picker, but that is a UI affordance, not a wire constraint other clients share.Page-size and thumbnail defaults are this client's choice, called out as such in the code: WA Web derives them at each call site from the surface it is rendering, and no A/B prop or bundle constant backs them.
Media
Not covered — reported rather than improvised.
biz-cover-photois a real media type in WA Web's list, and the upload returns{ fbid, meta_hmac, ts }, which the IQ quotes back ascover_photo@id/token/ts. ButRawUploadResponseinsrc/upload.rsrequiresurl+direct_path, which that endpoint does not return, so the existing pipeline cannot perform this upload — a genuine sub-gap, not something to fake with a new media type.So
set_cover_phototakes aCoverPhotoUploadreceipt instead of image bytes: the IQ is a complete, tested protocol contract, and closing the upload gap later makes it callable end-to-end without redesign.remove_cover_photoneeds no upload and works today.Checked and not changed
product_catalog_add/_edit/_deletereturn zero hits across all 497 bundles, and there is no MEX mutation for them either. Only Baileys has these; they are not a WA Web operation. Implementing them would mean shipping a wire format with no ground truth behind it, and their image path depends on the upload gap above. Deferred deliberately, per the task's "half-delivery" guidance.report_product,graph_ql_product_catalog_get_public_key,biz_create_order— confirmed real, out of scope, no consumer.QueryProductListCatalog— real, and interesting (the last live catalog IQ, with a gated MEX twin), but it is a by-id status query, not catalog listing.Validation
36 new tests: request construction and response parsing per operation, malformed/missing/unknown-field failures as typed errors, delta values rejected before serialization (non-finite and out-of-range coordinates, opening times outside the day, with midnight-crossing ranges asserted still accepted), and a precision test using 2^53+1 thousandths that fails if the amount ever passes through an
f64. Fixtures are hand-written from the response shapes, with fictitious ids, names and JIDs.Full CI was green on
563856c(26/26). Since then this branch mergedmain(8e59de5) — both sides had appended to the same sorted re-export list insrc/lib.rs, resolved as the union so neither the business types nor upstream'sNewsletterAdminInfo/NewsletterAdminProfile/NewsletterFollowerare lost. The counts above are post-merge.cargo clippy --workspacecannot run in this container —alsa-sysfails to build for want of a system ALSA dev package (CI installs it explicitly; the dependency comes from thevoip-cliexample, unrelated to this change). Scoped to the two crates touched instead, with--all-featuresto match what CI enforces.cargo-nextestwas likewise unavailable, socargo testwas used.