Leverage the exact schema from Reflect for a more precise mcap export.#797
Conversation
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a4c9fb4e38
ℹ️ 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".
| if let Some(type_info) = field.type_info() | ||
| && let TypeInfo::Enum(enum_info) = type_info | ||
| { | ||
| return enum_info.variant("None").is_some() && enum_info.variant("Some").is_some(); |
There was a problem hiding this comment.
Restrict Option detection to real Option enums
The new optional-field heuristic treats any enum that has None and Some variants as an Option, which can silently generate incorrect schemas for normal domain enums. In those cases, schema_for_option_enum collapses the enum to null | <Some payload> and drops other variants, and struct fields using that enum are marked non-required, so emitted payloads can fail validation or be documented inaccurately.
Useful? React with 👍 / 👎.
| let metadata_channel_id = mcap_writer | ||
| .add_channel( | ||
| metadata_schema_id, | ||
| &format!("/{task_id}/__meta"), |
There was a problem hiding this comment.
Count metadata channels in export statistics
Each task now registers an additional /{task_id}/__meta channel, but channels_created is still derived from one ChannelInfo entry per task, so the reported count underestimates the actual MCAP channel count by roughly 2x. This makes McapExportStats and CLI output misleading for monitoring or sanity checks that rely on channel totals.
Useful? React with 👍 / 👎.
Summary
Related issues
Changes
Testing
just std-cijust lintcargo +stable nextest run --workspace --all-targetsChecklist
Breaking changes (if any)
Additional context