Repository navigation
Consume contextual group notification contracts and owned normalization - #1817
Conversation
|
Caution Review failedThe pull request is closed. Finishing Touches
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 |
|
@codex review Please review this draft on head ec54c30. Focus on independent whatspec pin/digest and oracle provenance, typed PN/LID validation, failure precedence, participant_pn recovery, username/history gates, admin implications, and API compatibility. The contextual adapter is deliberately opt-in and incomplete; raw create/link nodes must not be mistaken for full normalization, and runtime adoption remains pending. |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
[Medium impact] No code issue found in this review blocks merging, though the PR remains draft pending its stated qualification and CI checks.
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ec54c30959
ℹ️ 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".
|
@codex review Review final head da98323. Only the immutable upstream qualifier pin/header changed since the initial implementation: upstream now tests an independent official input matrix and rejects two deliberate IR mutations. Regeneration and --check passed again; all other artifacts are unchanged. The wider generated envelope/create/link/runtime work is still explicitly pending, so this draft must not merge yet. |
|
Codex Review: Didn't find any major issues. Another round soon, please! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
📦 Binary size report
Heavy diagnostics were omitted from this gate-only measurement. The size gate uses the linked binary measurements above. Baseline: |
|
@codex review Please review head 20482b8. This completes the previously partial UNLINK adapter with generated ordered envelope/LINK parsing, owned typed actions, original-JS conformance and opt-in runtime facts. It pins source ae4ea6e from whatspec#59. Existing review concerns about tag dispatch, field-specific username gates and unknown roles are covered by complete parsers and tests. Please review the complete final diff; keep draft pending CI/review. |
|
@codex review Final head is now 20ac098. The follow-up only regenerates the missing tag-constant provenance comment from the newer preserved evidence already on this branch; implementation remains 20482b8. Review the complete diff. Both previous Codex threads now have source/test evidence and are resolved. |
|
Codex Review: Didn't find any major issues. You're on a roll. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
Merging this PR will degrade performance by 19.47%3 warnings
Warning Please fix the performance issues or acknowledge them on CodSpeed. Performance Changes
Tip Investigate this regression by commenting Comparing Footnotes
|
|
@codex review Please review final head 30647b4. The complete normalized implementation in 20ac098 was reviewed without findings. Follow-ups preserve that work, fix generated boolean assertions so all-target Clippy passes, and align all four overlay pins to WhatSpec 3d4737934a55239928b3265523448aef2978a82b. The three contract fixtures are byte-identical to the producer; the main IR pin is unchanged. Local validation passed: 83 group parser tests, 127 codegen unit tests plus four checks, three runtime-facts tests with generated-group-notifications enabled, wacore/codegen all-target Clippy, formatting and all 22 generated text artifacts. E2E, binary size and CodSpeed passed on the previous 97af08d head with identical runtime implementation. Please inspect the complete final diff, including opt-in runtime facts, owned actions, legacy compatibility and source qualification. Both PRs stay draft pending final CI/review. |
|
Codex Review: Didn't find any major issues. Hooray! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
@codex review Please review c7d9607. The prior review of 30647b4 completed without findings. This follow-up only advances the four immutable overlay provenance pins and generated comments to WhatSpec ed8da49c24a0e943854e1c7077c60129e169aeb4. All three contract fixtures remain byte-identical; no runtime implementation or oracle inputs changed. All 22 generated text artifacts match codegen --check. The runtime implementation retains its 83 group tests, 127 codegen tests plus four checks, three feature-enabled runtime-facts tests and all-target Clippy validation. Final CI and producer review still gate readiness. |
|
Codex Review: Didn't find any major issues. Delightful! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
@codex review Please review final head 838ac4d. The prior c7d9607 review completed without findings. This follow-up only advances the four immutable overlay provenance pins and generated comments to WhatSpec 57494ccd865fa6471743264fea036f00d26d89cb, which now rejects unqualified opaque source effects. All three contracts are byte-identical to the producer; runtime implementation, fixtures and oracle inputs are unchanged. All 22 generated artifacts match codegen --check, formatting passes, and the unchanged runtime implementation retains its 83 group tests, three feature-enabled runtime-facts tests, 127 codegen tests plus four checks and all-target Clippy validation. Please check the final full diff and immutable provenance linkage. CI and the producer's final review remain required before readiness. |
|
Codex Review: Didn't find any major issues. Can't wait for the next one! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
@codex review Please review final head d942ebe. The prior 838ac4d review completed without findings; its E2E and autofix workflows passed. This follow-up only advances the four immutable overlay pins and generated provenance comments to WhatSpec adbe317bf0320bb0cb83824722f6eb60c2ac69da, whose completed review also found no major issues. All three contracts are byte-identical to the producer; runtime code, fixtures and oracle inputs are unchanged. All 22 generated text artifacts match codegen --check, formatting passes and producer wa-notif validation is now 131 tests plus all-target/all-feature Clippy. Please verify the final full diff and provenance linkage. Final CI remains pending and both PRs stay draft; no merge. |
|
Codex Review: Didn't find any major issues. Delightful! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
Group notifications currently expose raw create/link/unlink shapes and parse without the real account facts. This adds owned typed normalization, generated ordered envelope guards and LINK/UNLINK decisions, plus a compatibility adapter preserving the public event API.
The independent contract overlay pins WhatSpec adbe317bf0320bb0cb83824722f6eb60c2ac69da from oxidezap/whatspec#59. Contract bytes and SHA-256 digests are verified. The main IR pin/schema and package dependency graph are unchanged. Envelope and LINK/UNLINK code are generated static Rust; complex CREATE and other action payloads are handwritten adapters qualified against independent original-source fixtures. No runtime JS is introduced.
The nondefault generated-group-notifications feature and an installed GroupNotificationFactsProvider opt into runtime adoption. Effective username/history/opus flags and country lookup come from the provider; exact local device PN/LID identities come from the cached device snapshot. Without a provider, the existing parser remains active. Cache, session rotation, ACK and storage effects remain in existing handlers.
Coverage includes exact destination identity, locally recovered invalid participant_pn, field-specific username gates, history gates, participant defaults/superadmin mapping, typed unknown actions, marker flags without actions, request methods, parent/subgroup integrity and error ordering. Inherited Object.prototype results and unsupported numeric/legacy ranges reject explicitly. Source qualification uses explicit account/Wid/country/telemetry/binary mocks and no authenticated session.
Runtime implementation validation: 83 group tests; three feature-enabled client runtime-facts tests; 127 codegen unit tests plus four checks; pinned-nightly all-target Clippy for wacore/codegen; formatting. The library check without default features with tokio-runtime and the new feature passed; existing library tests require default SQLite/transport features.
Head d942ebe advances the four overlay provenance pins and generated comments to the producer head reviewed without findings. Runtime code, all three contract fixtures and oracle inputs are unchanged. All 22 generated artifacts match codegen --check and formatting passes. Codex reviewed 838ac4d without findings; its E2E and autofix workflows passed. Codex completed final-head d942ebe review without findings. Final-head CI remains pending.
Current CI blocker: API Consumers job Independent API hosts (msrv) failed on d942ebe. Fresh dependency resolution for tests/sqlite-interop (bundled/sqlcipher) and tests/sqlite-sdk-browser selected lock_api 0.5.0, parking_lot 0.13.0 and parking_lot_core 0.10.0, which require Rust 1.95; the job uses Rust 1.94.1. This needs separate dependency/MSRV coordination before readiness. Evidence: https://github.com/oxidezap/whatsapp-rust/actions/runs/40953685196/job/123728253723. Other final-head workflows are still queued/running; autofix passed.
Depends on the producer contract PR. Keep draft pending final qualification; no merge is requested.