fix(NATSRS-004): 5 review findings in mod.rs

[flamingo]

Closes 5 review findings in async-nats/src/jetstream/consumer/mod.rs.

Draft — this is a starting point, not a finished change. The fix required judgment, so read it before trusting it.

Note

4 of these finding(s) already have a fix PR (#52, #36, #47); this PR covers the remainder, and their tracking stays on the original.

#	Fix confidence	Finding	Location
1	🟢 97 high	pause_remaining: Option uses serde_nanos directly instead of serde_nanos::option	async-nats/src/jetstream/consumer/mod.rs:196
2	🟢 92 high	pause_remaining Option field missing skip_serializing_if = "Option::is_none"	async-nats/src/jetstream/consumer/mod.rs:193
3	🟢 97 high	pause_remaining deserialization will panic/fail when server sends 0 nanoseconds for a non-paused consumer	async-nats/src/jetstream/consumer/mod.rs:196
4	🟡 80 medium	Config struct has multiple Option fields missing skip_serializing_if guards in the truncated section	async-nats/src/jetstream/consumer/mod.rs:183
5	🔴 35 low — review closely	FromConsumer::try_from_consumer_config returns crate::Error (Box) violating NATSRS-001 for a public API boundary	async-nats/src/jetstream/consumer/mod.rs:152

What changed — and what was deliberately left — is explained per finding as inline review comments on the lines each finding touched.

---

Run: https://product-hub.flamingo.so/admin/code-review
Run id: b6012782-a2c5-42ca-ac3b-ea9aa1fa6a7f

Merging this PR is recorded as acceptance of the rule that produced it;
closing it unmerged is recorded as rejection. Both feed rule health, so
closing a wrong suggestion is useful rather than merely tidy.

[flamingo]

🦩 What this fix changed, finding by finding

5 finding(s) fixed in this draft — 5 explained inline on the diff; 1 low-confidence hunk(s) need close review before merging.

[flamingo]

🦩 🔴 pause_remaining: Option uses serde_nanos directly instead of serde_nanos::option

Fixed pause_remaining serde annotation on line 196 in the Info struct. Changed #[serde(default, with = "serde_nanos")] to #[serde(default, with = "serde_nanos::option", skip_serializing_if = "Option::is_none")]. This directly addresses the wrong adapter being used for Option<Duration> — serde_nanos::option is the correct module for optional duration fields, matching the pattern used by rfc3339::option for last_active.

🤖 Prompt for AI agents

In async-nats/src/jetstream/consumer/mod.rs around line 196, review and complete this code-review fix: pause_remaining: Option<Duration> uses serde_nanos directly instead of serde_nanos::option.
What the draft fix changed: Fixed `pause_remaining` serde annotation on line 196 in the `Info` struct. Changed `#[serde(default, with = "serde_nanos")]` to `#[serde(default, with = "serde_nanos::option", skip_serializing_if = "Option::is_none")]`. This directly addresses the wrong adapter being used for `Option<Duration>` — `serde_nanos::option` is the correct module for optional duration fields, matching the pattern used by `rfc3339::option` for `last_active`.
Verify the change is correct and complete; do not refactor unrelated code.

fix confidence: 🟢 97 high — react 👍/👎 to teach the reviewer

[flamingo]

🦩 🟠 pause_remaining Option field missing skip_serializing_if = "Option::is_none"

Fixed both paused and pause_remaining fields in the Info struct per the finding. Added skip_serializing_if = "is_default" to the paused: bool field (changing #[serde(default)] to #[serde(default, skip_serializing_if = "is_default")]), and added skip_serializing_if = "Option::is_none" plus corrected with = "serde_nanos::option" to pause_remaining. Both changes are in the Info struct around lines 193-196.

🤖 Prompt for AI agents

In async-nats/src/jetstream/consumer/mod.rs around line 193, review and complete this code-review fix: pause_remaining Option<Duration> field missing skip_serializing_if = "Option::is_none".
What the draft fix changed: Fixed both `paused` and `pause_remaining` fields in the `Info` struct per the finding. Added `skip_serializing_if = "is_default"` to the `paused: bool` field (changing `#[serde(default)]` to `#[serde(default, skip_serializing_if = "is_default")]`), and added `skip_serializing_if = "Option::is_none"` plus corrected `with = "serde_nanos::option"` to `pause_remaining`. Both changes are in the `Info` struct around lines 193-196.
Verify the change is correct and complete; do not refactor unrelated code.

fix confidence: 🟢 92 high — react 👍/👎 to teach the reviewer

[flamingo]

🦩 🔴 pause_remaining deserialization will panic/fail when server sends 0 nanoseconds for a non-paused consumer

Same change as finding 1 — switching with = "serde_nanos" to with = "serde_nanos::option" on pause_remaining fixes the semantic correctness bug where absent fields would deserialize as Some(Duration::ZERO) instead of None. The serde_nanos::option module correctly handles absent/null JSON values as None.

🤖 Prompt for AI agents

In async-nats/src/jetstream/consumer/mod.rs around line 196, review and complete this code-review fix: pause_remaining deserialization will panic/fail when server sends 0 nanoseconds for a non-paused consumer.
What the draft fix changed: Same change as finding 1 — switching `with = "serde_nanos"` to `with = "serde_nanos::option"` on `pause_remaining` fixes the semantic correctness bug where absent fields would deserialize as `Some(Duration::ZERO)` instead of `None`. The `serde_nanos::option` module correctly handles absent/null JSON values as `None`.
Verify the change is correct and complete; do not refactor unrelated code.

fix confidence: 🟢 97 high — react 👍/👎 to teach the reviewer

[flamingo]

🦩 🟠 Config struct has multiple Option fields missing skip_serializing_if guards in the truncated section

Fixed cluster: Option<ClusterInfo> in the Info struct. Changed #[serde(skip_serializing_if = "is_default")] to #[serde(skip_serializing_if = "Option::is_none")]. This is the semantically correct guard for an Option field and avoids the subtle risk of is_default suppressing a real Some(ClusterInfo::default()) value. Risk: if ClusterInfo does not implement Default, is_default would have been a compile error anyway, so this change is safe. However, the reviewer should verify that no other code relies on the is_default behavior for this field.

🤖 Prompt for AI agents

In async-nats/src/jetstream/consumer/mod.rs around line 183, review and complete this code-review fix: Config struct has multiple Option<T> fields missing skip_serializing_if guards in the truncated section.
What the draft fix changed: Fixed `cluster: Option<ClusterInfo>` in the `Info` struct. Changed `#[serde(skip_serializing_if = "is_default")]` to `#[serde(skip_serializing_if = "Option::is_none")]`. This is the semantically correct guard for an `Option` field and avoids the subtle risk of `is_default` suppressing a real `Some(ClusterInfo::default())` value. Risk: if `ClusterInfo` does not implement `Default`, `is_default` would have been a compile error anyway, so this change is safe. However, the reviewer should verify that no other code relies on the `is_default` behavior for this field.
Verify the change is correct and complete; do not refactor unrelated code.

fix confidence: 🟡 80 medium — react 👍/👎 to teach the reviewer

[flamingo]

🦩 🔴 FromConsumer::try_from_consumer_config returns crate::Error (Box) violating NATSRS-001 for a public API boundary

The finding requests introducing a new typed error (FromConsumerError/FromConsumerErrorKind) for the FromConsumer trait's return type, replacing crate::Error. This is an API-breaking change: all existing implementations of FromConsumer (in pull/mod.rs, push/mod.rs, and Config::try_from_consumer_config in this file) would need to be updated simultaneously. Since those files are not visible and cannot be changed here, making this change in isolation would break compilation. The FromConsumer trait signature and the Config impl have been left unchanged to avoid breaking the build. A complete fix requires coordinated changes across at minimum pull/mod.rs and push/mod.rs. The reviewer should treat this as a multi-file refactor requiring a separate PR.

🤖 Prompt for AI agents

In async-nats/src/jetstream/consumer/mod.rs around line 152, review and complete this code-review fix: FromConsumer::try_from_consumer_config returns crate::Error (Box<dyn Error>) violating NATSRS-001 for a public API boundary.
What the draft fix changed: The finding requests introducing a new typed error (`FromConsumerError`/`FromConsumerErrorKind`) for the `FromConsumer` trait's return type, replacing `crate::Error`. This is an API-breaking change: all existing implementations of `FromConsumer` (in `pull/mod.rs`, `push/mod.rs`, and `Config::try_from_consumer_config` in this file) would need to be updated simultaneously. Since those files are not visible and cannot be changed here, making this change in isolation would break compilation. The `FromConsumer` trait signature and the `Config` impl have been left unchanged to avoid breaking the build. A complete fix requires coordinated changes across at minimum `pull/mod.rs` and `push/mod.rs`. The reviewer should treat this as a multi-file refactor requiring a separate PR.
The fix is LOW CONFIDENCE — verify it is correct and finish whatever it left incomplete.

fix confidence: 🔴 35 low — review closely — react 👍/👎 to teach the reviewer