P1: coverage gap-fills — proto validation guard, wire enums, message/receipt mixin catalog, conditional children

[jlucaso1]

Summary

Follow-up to #22 (P0) and #23 (P1 pipeline-trust). This closes out the remaining P1 items — but the re-evaluation changed what they are. The original framing ("build new message/receipt domains, fix proto correctness") didn't survive empirical validation:

• New message/receipt domains would duplicate what stanza/incoming/srvreq already extract — the genuinely-uncaptured slice is the mixin fragments the pipeline deliberately drops.
• Proto extraction is provably faithful already (verified field-by-field; the committed .proto compiles under protoc 35 and protox). The real hole was the absence of a validation guard, not extraction correctness.

So this PR is four targeted gap-fills, each validated against the real bundle and shipped as its own commit.

What's in it

1. Proto validation guard (test(proto))
WAProto.proto was the only generated artifact with no CI validation (JSON domains are schema-checked; the .proto was never even parsed). Since update.yml auto-regenerates and auto-commits on new WA versions with no review, a future extraction break (dangling type ref, dup name, mis-scoped enum) would land silently. Adds protox (pure-Rust protoc) as a wa-proto dev-dependency and a test that compiles the committed WAProto.proto, resolving every cross-reference. C-free (no ring/aws-lc/openssl-sys) and dev-only, so it touches neither the wasm-safe wa-ir contract nor the C-free CI guard. A missing .proto hard-fails under CI (so the guard can't be silently skipped) while tolerating local sparse checkouts.

2. Plain-object wire enums (feat(enums)) — enumDefs 257 → 288
Some wire enums are plain object literals with no $InternalEnum dep, so the catalog never saw them: WAWebAck.ACK/ACK_STRING/EDIT_ATTR, WAAckLevel.ACK, and the WASmaxIn…Enums bags (receipt, message-deliver/publish, account, biz payment/marketing/settings). A plain object is ambiguous — an untargeted scan pulls in ~190 infra constants (loggers, locales, JPEG markers, .pb proto-duplicates) — so the pass is gated on WA's own protocol enum-bag naming convention (WASmaxIn…Enums / WASmaxOut…Enums + the two ack-level modules) and keeps only CONSTANT_CASE members. Result: exactly +31 genuine protocol enums, 0 noise.

3. Message/receipt content-type mixin catalog (feat(stanza)) — stanzaDefs 84 → 174
The stanza scanner dropped every merge…Mixin module whole, so the smax message/receipt content-type surface (which WA composes at runtime from per-type mixin fragments — {type:"reaction"}, media, poll, quiz, revoke; receipt view/newsletter/status-class) was invisible. These are now kept, marked with a new StanzaDef.fragment flag (default false, omitted when serialized). Adds 90 catalogued fragments, including the receipt view-publish mixins a newsletter-view receipt request composes. Consumers building a sendable stanza filter fragment: true out.

The fragment classifier uses any(merge…Mixin), not all (matching the iq scanner after #23's P1-A fix): a mixin that also exports a make… helper was otherwise misclassified as a standalone sendable stanza. Net split after this fix: 77 standalone + 97 fragment = 174 (7 WASmaxOut*Mixin modules correctly moved standalone→fragment; the standalone content itself is otherwise unchanged).

4. Conditional (ternary) stanza children (feat(scan))
resolve_child_node traced variable refs, arrays, and .map() templates but not a cond ? A : B child, so any conditionally-attached child was dropped — e.g. the aggregate receipt/ack builder's a.length > 0 ? wap("list", …) : null. Now resolves both branches and unions their children (dedup'd): the common cond ? node : null yields just the node, while a genuinely divergent cond ? A : B catalogs both shapes rather than silently dropping one. Purely additive: iq gains conditional children (<gdpr> variants, <picture type=image>) and 8 stanza builders gain children (the aggregate ack's <list><item>, receipt <meta>/<bot>, message <meta>/<key>/<sender_content_binding>/<interactive>/<native_flow>). Shared with the iq path; verified the iq delta is exactly those additions and nothing else.

Validation

Every output-changing commit was regenerated against the real bundle (version pinned) and diffed:

• Enums: +31, hand-verified all are protocol (not infra), 0 removed.
• Stanza fragments: standalone content unchanged (bar the 7 correctly-reclassified mixins), 0 structureless fragments, determinism --check byte-identical.
• Conditional children: additive only — no child removed from any iq or stanza entry.

Full CI-equivalent gate is green locally: fmt, clippy --workspace --all-targets --all-features -D warnings, cargo test --workspace (32 suites), wa-ir --features schema, wa-ir/wa-fetch wasm32, whatspec --no-default-features, C-free, validate-schemas.py (11 domains), and the byte-identical determinism check.

🤖 Generated with Claude Code

https://claude.ai/code/session_013111jePsSca7epxMugRn2A

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

[coderabbitai]

📝 Walkthrough

Walkthrough

Adds a fragment boolean field to StanzaDef, updating the stanza scanner to retain and flag fragment-module stanzas instead of dropping them, and updates codegen tests accordingly. Adds ternary/conditional child-node resolution to the request scanner, extends enum extraction to whitelisted plain-object protocol modules, and adds a protox-based proto compile validation test.

Changes

Stanza Fragment Tracking

Layer / File(s)	Summary
Fragment field contract crates/wa-ir/src/iq.rs	Adds pub fragment: bool to StanzaDef with serde default and skip-if-false behavior.
Scanner fragment classification crates/wa-scan/src/stanza.rs	scan_module now keeps stanzas from fragment modules (merge…Mixin combinators) and marks each StanzaDef with fragment status instead of discarding them; adds/updates tests for conditional children, fragment detection, and standalone stanzas.
Codegen test literal updates crates/wa-codegen/src/stanza_export.rs	Updates existing test StanzaDef literals to include the new fragment: false field.

Conditional Child Node Resolution

Layer / File(s)	Summary
Ternary branch resolution crates/wa-scan/src/request.rs	resolve_child_node resolves the consequent branch of conditional expressions first and falls back to the alternate branch, preserving conditional wap() subtrees.

Protocol Enum Extraction

Layer / File(s)	Summary
Plain-object protocol enum extraction crates/wa-enums/src/lib.rs	Adds is_constant_case, is_protocol_enum_module, and extract_plain_object_enums to capture named enum exports from whitelisted wire-protocol modules lacking $InternalEnum, merges results into the deduped enum catalog, and adds tests for inclusion/exclusion cases.

Proto Compile Validation

Layer / File(s)	Summary
protox dependency and compile test crates/wa-proto/Cargo.toml, crates/wa-proto/tests/proto_compiles.rs	Adds protox dev-dependency and a new test that compiles/resolves the committed WAProto.proto, skipping if absent and asserting a non-empty descriptor set.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Possibly related PRs

• oxidezap/whatspec#2: Both PRs modify resolve_child_node in crates/wa-scan/src/request.rs for improved child-node recovery.
• oxidezap/whatspec#15: Both PRs modify fragment-module handling in crates/wa-scan/src/stanza.rs, one dropping fragment modules and this one flagging them instead.
• oxidezap/whatspec#17: Both PRs touch crates/wa-codegen/src/stanza_export.rs in relation to the StanzaDef/stanza IR shape.

Suggested reviewers: greptile-apps, cubic-dev-ai

Poem

A flag called fragment, small and neat,
Keeps mixin stanzas from retreat 🐇
Ternary paths no longer stray,
Protocol enums join the fray,
Proto compiles, checked with care—
Hop hop hooray, the code's all there! 🥕

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately captures the four main coverage-gap additions in the PR, though it is a bit verbose.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 4

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

crates/wa-scan/src/stanza.rs (1)

69-72: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Stale comment: fragment modules are no longer "dropped whole in scan_module".

This comment still asserts the old behavior. scan_module now keeps fragment-module stanzas and tags them via fragment; only the generic structureless filter below removes them (if they truly have no attrs/children/subtype). Update the comment to avoid confusing future readers about the fragment classification.

✏️ Suggested comment fix

-    // Drop structureless stanzas (no attrs, no children, no subtype) — a bare `<ack/>`
-    // carries nothing a consumer can model. (Fragment `merge…Mixin` modules are already
-    // dropped whole in `scan_module`.)
+    // Drop structureless stanzas (no attrs, no children, no subtype) — a bare `<ack/>`
+    // carries nothing a consumer can model. Fragment `merge…Mixin` modules are now kept
+    // and marked via `StanzaDef::fragment`, so they're only dropped here if genuinely
+    // structureless.

🤖 Prompt for 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.

In `@crates/wa-scan/src/stanza.rs` around lines 69 - 72, The comment in
`stanza.rs` is stale because `scan_module` no longer drops fragment modules
wholesale; update the note above the `out.retain` filter to describe the current
behavior in `scan_module` and `merge…Mixin` handling, making clear that
fragment-module stanzas are kept and tagged via `fragment`, while this filter
only removes truly structureless stanzas with no attrs, children, or subtype.
Keep the comment aligned with the logic around `out.retain` and `scan_module` so
future readers are not misled about fragment classification.

🤖 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 `@crates/wa-enums/src/lib.rs`:
- Around line 188-198: Add a regression test for extract_plain_object_enums that
covers a plain-object export with mixed literal kinds, such as one int and one
string variant, and verify it is excluded from the extracted enums. Use the
existing plain-object enum parsing path in crates/wa-enums/src/lib.rs and mirror
the mixed-skip expectation already covered by
default_export_named_by_module_and_mixed_skipped so the shared parsing logic is
protected.
- Around line 175-187: The pending-resolution loop in the resolver is duplicated
between this code path and resolve_named_enum, so factor the shared logic into a
helper on NamedResolver (for example, resolve_pending) and call it from both
places. Move the iteration over r.pending and the r.locals/r.exports update into
that helper, keeping the behavior identical while removing the duplicated block.

In `@crates/wa-scan/src/request.rs`:
- Around line 219-249: The conditional-child branch in resolve_child_node adds
two more repeated recursive calls with the same long argument list. Refactor the
local recursion pattern by introducing a small helper closure or local wrapper
that captures the shared context arguments (node_source, scope, module_source,
aliases, contributions, helpers, depth), then use it for the
Expression::ConditionalExpression path and the existing similar cases in
resolve_child_node to reduce boilerplate and make the function easier to read.
- Around line 219-249: The conditional handling in resolve_child_node currently
returns the consequent branch immediately when it resolves non-empty, which can
drop a valid alternate branch for ternaries like cond ? wap("a", …) : wap("b",
…). Update the Expression::ConditionalExpression path to inspect both
cond.consequent and cond.alternate in the same logic, and merge children when
both sides resolve to non-empty instead of early-returning. Keep the existing
fallback behavior for cond ? node : null / cond ? null : node, but ensure
genuinely divergent branches in request.rs are both preserved.

---

Outside diff comments:
In `@crates/wa-scan/src/stanza.rs`:
- Around line 69-72: The comment in `stanza.rs` is stale because `scan_module`
no longer drops fragment modules wholesale; update the note above the
`out.retain` filter to describe the current behavior in `scan_module` and
`merge…Mixin` handling, making clear that fragment-module stanzas are kept and
tagged via `fragment`, while this filter only removes truly structureless
stanzas with no attrs, children, or subtype. Keep the comment aligned with the
logic around `out.retain` and `scan_module` so future readers are not misled
about fragment classification.

🪄 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: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: df69b57a-64f8-4ca9-84ad-ab77d8c773a7

📥 Commits

Reviewing files that changed from the base of the PR and between 3566d2e and e7dcc81.

⛔ Files ignored due to path filters (6)

• Cargo.lock is excluded by !**/*.lock
• generated/enums/index.json is excluded by !**/generated/**
• generated/iq/index.json is excluded by !**/generated/**
• generated/manifest.json is excluded by !**/generated/**
• generated/schema/stanza.schema.json is excluded by !**/generated/**
• generated/stanza/index.json is excluded by !**/generated/**

📒 Files selected for processing (7)

• crates/wa-codegen/src/stanza_export.rs
• crates/wa-enums/src/lib.rs
• crates/wa-ir/src/iq.rs
• crates/wa-proto/Cargo.toml
• crates/wa-proto/tests/proto_compiles.rs
• crates/wa-scan/src/request.rs
• crates/wa-scan/src/stanza.rs

[greptile-apps]

Greptile Summary

This PR adds four targeted coverage gap-fills: a protox-based compile guard for WAProto.proto, a plain-object wire-enum catalog pass scoped to WA's protocol-naming convention (+31 enums), merge…Mixin fragment stanza capture with a new fragment flag (+90 fragments), and conditional (cond ? A : B) child resolution that now unions both branches instead of dropping the alternate.

• Proto guard (wa-proto/tests/proto_compiles.rs): adds a dev-only protox dependency and a test that parses and fully resolves WAProto.proto; correctly asserts a hard failure under CI env if the file is missing, while silently skipping in local sparse checkouts.
• Plain-object enum detection (wa-enums/src/lib.rs): gated on WA's WASmax<In|Out>…Enums / WAWebAck / WAAckLevel module-name convention and filtered to CONSTANT_CASE-only members, keeping the catalog noise-free; resolve_pending is cleanly extracted and shared between resolve_named_enum and the new catalog pass.
• Fragment mixin catalog (wa-scan/src/stanza.rs): is_fragment_module switches from all-exports-are-mixins to any-export-is-a-mixin, broadening classification; fragment is set uniformly for every stanza in the module; the PR asserts the non-fragment set is byte-identical to before (no standalone stanzas reclassified).

Confidence Score: 5/5

Safe to merge; all four gap-fills are additive, the non-fragment stanza set is byte-identical to before, and every new code path is covered by tests including both-branches and no-mixin cases.

The enum, stanza, and ternary changes are all additive and fully validated against the real bundle. The is_fragment_module broadening from all→any carries a documented assumption about WA's architecture, but the determinism check confirms zero regressions in the current bundle. The proto guard correctly hard-fails under CI. No correctness bugs found.

crates/wa-scan/src/stanza.rs — the any-wins fragment classification rule; worth a second read if a future WA update significantly reorganises its mixin-export conventions.

Important Files Changed

Filename	Overview
crates/wa-enums/src/lib.rs	Adds plain-object enum extraction gated on WA's protocol-module naming convention; resolve_pending correctly extracted and shared; CONSTANT_CASE filter and ≥2 members guard prevent noise; tests cover all boundary cases.
crates/wa-scan/src/stanza.rs	Switches is_fragment_module from all→any and marks all stanzas in a module uniformly; PR validates standalone set is byte-identical to before, but the any rule could silently misclassify a future WA module that exports both a mixin and a standalone builder.
crates/wa-scan/src/request.rs	Adds ternary child resolution that unions both branches; passes existing depth (not depth+1) consistently with other non-function-call cases; recursion is bounded by existing MAX_FN_DEPTH guards in the function-trace helpers; well-covered by new tests.
crates/wa-proto/tests/proto_compiles.rs	Proto compile guard correctly panics under CI when the file is absent; uses protox::compile with include-root for full cross-reference resolution; sanity-checks a non-empty descriptor set.
crates/wa-ir/src/iq.rs	Adds fragment: bool field to StanzaDef with serde(default, skip_serializing_if = "Not::not") so existing 84 standalone stanzas remain byte-identical in serialized form.
crates/wa-codegen/src/stanza_export.rs	Test fixtures updated to include fragment: false on existing StanzaDef structs; no logic changes.
crates/wa-proto/Cargo.toml	Adds protox = "0.9" as a dev-dependency only; C-free and wasm-safe surface unaffected.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Bundle Source] --> B[extract_module_definitions]
    B --> C{Module type?}
    C -- has $InternalEnum dep --> D[extract_from_module\nInternalEnum catalog path]
    C -- is_protocol_enum_module\nWASmaxIn/OutEnums\nWAWebAck / WAAckLevel --> E[extract_plain_object_enums\nCONSTANT_CASE filter\n≥2 members guard]
    C -- other --> F[skip]
    D --> G[EnumsIr\n257→288 enumDefs]
    E --> G

    B --> H[scan_stanzas_from_modules]
    H --> I{is_fragment_module?\nany export starts with merge + Mixin}
    I -- yes fragment=true --> J[StanzaDef fragment=true\n90 new mixin fragments]
    I -- no fragment=false --> K[StanzaDef fragment=false\n84 existing standalone stanzas]
    J --> L[retain: not structureless]
    K --> L
    L --> M[StanzaIr\n84→174 stanzaDefs]

    H --> N[resolve_child_node]
    N --> O{Expression type?}
    O -- Parenthesized --> N
    O -- ConditionalExpression\ncond ? A : B --> P[resolve consequent\n+ resolve alternate\nunion deduplicate]
    O -- wap/smax call --> Q[WapChildNode]
    O -- Array --> R[flatten elements]
    O -- Identifier ref --> S[trace var initializer]
    O -- .map call --> T[resolve map template]
    P --> Q
    R --> Q
    S --> Q
    T --> Q

    U[WAProto.proto] --> V[protox::compile\nparse + resolve all cross-refs]
    V -- failure --> W[panic: broken schema]
    V -- success --> X[assert non-empty descriptor set]

Loading

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A[Bundle Source] --> B[extract_module_definitions]
    B --> C{Module type?}
    C -- has $InternalEnum dep --> D[extract_from_module\nInternalEnum catalog path]
    C -- is_protocol_enum_module\nWASmaxIn/OutEnums\nWAWebAck / WAAckLevel --> E[extract_plain_object_enums\nCONSTANT_CASE filter\n≥2 members guard]
    C -- other --> F[skip]
    D --> G[EnumsIr\n257→288 enumDefs]
    E --> G

    B --> H[scan_stanzas_from_modules]
    H --> I{is_fragment_module?\nany export starts with merge + Mixin}
    I -- yes fragment=true --> J[StanzaDef fragment=true\n90 new mixin fragments]
    I -- no fragment=false --> K[StanzaDef fragment=false\n84 existing standalone stanzas]
    J --> L[retain: not structureless]
    K --> L
    L --> M[StanzaIr\n84→174 stanzaDefs]

    H --> N[resolve_child_node]
    N --> O{Expression type?}
    O -- Parenthesized --> N
    O -- ConditionalExpression\ncond ? A : B --> P[resolve consequent\n+ resolve alternate\nunion deduplicate]
    O -- wap/smax call --> Q[WapChildNode]
    O -- Array --> R[flatten elements]
    O -- Identifier ref --> S[trace var initializer]
    O -- .map call --> T[resolve map template]
    P --> Q
    R --> Q
    S --> Q
    T --> Q

    U[WAProto.proto] --> V[protox::compile\nparse + resolve all cross-refs]
    V -- failure --> W[panic: broken schema]
    V -- success --> X[assert non-empty descriptor set]

Loading

_{Reviews (2): Last reviewed commit: "review: address cubic/greptile/coderabbi..." | Re-trigger Greptile}

[cubic-dev-ai]

5 issues found across 13 files

Confidence score: 3/5

• crates/wa-scan/src/request.rs still returns early in the ternary-child path, so when both branches build children the alternate can be skipped and valid schema entries are lost; merging as-is risks an incomplete stanza-child catalog for downstream consumers — update the branch handling to collect children from both consequent and alternate before returning.
• crates/wa-scan/src/stanza.rs can now classify mixin modules with helper exports as sendable stanzas (fragment: false), which may let partial/non-sendable stanzas pass fragment-based filters; that creates concrete misclassification risk in consumer logic — align the sendable-stanza predicate with the stricter scanner criteria before merge.
• In crates/wa-enums/src/lib.rs, the new plain-object enum path has weaker module-name gating and missing mixed-value rejection coverage, so false positives are more likely and regressions may slip through; the duplicated pending-resolution loop also increases drift risk — tighten the WASmaxIn...Enums/WASmaxOut...Enums predicate and add a mixed-value test (then optionally factor the shared loop in a follow-up).

<sub>Tip: cubic can generate docs of your entire codebase and keep them up to date. Try it here.

Re-trigger cubic</sub>

[cubic-dev-ai]

0 issues found across 7 files (changes from recent commits).

<sub>Requires human review: Major refactor of core extraction logic with a proto guard and new enum path; artifacts and schema change, and a bug could silently corrupt protocol metadata, so manual review is required.

Re-trigger cubic</sub>