Skip to content

docs(core): add compile-checked doctests for type-compatibility rules - #3108

Draft
phil-opp wants to merge 1 commit into
mainfrom
claude/dreamy-bardeen-xphg86-type-compat-doctests
Draft

docs(core): add compile-checked doctests for type-compatibility rules#3108
phil-opp wants to merge 1 commit into
mainfrom
claude/dreamy-bardeen-xphg86-type-compat-doctests

Conversation

@phil-opp

Copy link
Copy Markdown
Collaborator

Issue

libraries/core/src/types.rs exposes two public type-compatibility helpers whose behavior is subtle but was only documented in prose:

  • types_match — an unparameterized side is a wildcard, and parameter conflicts are only rejected on shared keys (disjoint keys never conflict).
  • CompatibilityGraph::is_compatible — the universal * -> std/core/v1/Bytes sink, same-base wildcard matching, and a depth-3 bound on transitive widening chains (BFS).

None of these rules were exercised by a compile-checked doctest, so a refactor could silently change them without any in-file test catching it. Sibling public APIs in the crate (e.g. config::ByteSize) already carry doctests.

Fix

Add # Examples doctests to both functions that pin the intended semantics:

  • types_match: identical match, unparameterized wildcard, disjoint-key compatibility, shared-key conflict, different-base mismatch.
  • is_compatible: Bytes sink, one-hop and two-hop (UInt8 -> UInt32 -> UInt64) built-in widening, same-base wildcard, and a four-rule chain demonstrating that three hops are reachable but four are beyond the depth limit.

Documentation only — no behavior change.

Validation

cargo test -p dora-core --doc      # 4 passed (2 new)
cargo clippy -p dora-core -- -D warnings
cargo fmt --all -- --check

⚠️ This is a machine-generated PR authored by Claude (Claude Code) as part of an automated codebase review. Each finding was verified by hand against origin/main before opening. Please review carefully before merging.

🤖 Generated with Claude Code


Generated by Claude Code

`types_match` and `CompatibilityGraph::is_compatible` encode several
non-obvious rules that were only described in prose: the wildcard
behavior of an unparameterized side, the "shared key must agree"
(vs. disjoint-key) parameter semantics, the universal `* -> Bytes`
sink, and the depth-3 bound on transitive widening chains.

Add `# Examples` doctests that pin each of these semantics so they
are compile-checked and cannot silently regress. Documentation only;
no behavior change.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PYiYgEo2WKBXfGooqP2WG5
@trunk-io

trunk-io Bot commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Merging to main in this repository is managed by Trunk.

  • To merge this pull request, check the box to the left or comment /trunk merge below.

After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here

Copy link
Copy Markdown
Collaborator Author

Automated codebase-review run — summary

This is the summary for a batch of machine-generated PRs from one automated review pass over origin/main (a8d3bbf). Each finding was verified by hand against current origin/main, checked against prior runs' open PRs for duplicates, and each branch was built off a fresh origin/main.

PRs opened, by category

Correctness

Documentation

Rejected / deferred findings (with reasons)

  • SHM copy-fallback over-allocation guard in zenoh_publishduplicate of open PR fix(node): guard the SHM copy-fallback against an over-allocated buffer #3073, which already guards that exact site.
  • MAVLink MavlinkArrow::schema() rebuilt per encode (perf) — real per-frame allocation, but a clean fix means changing the public trait method signature across ~17 impls; too large/API-breaking for an automated PR, better as a focused human change.
  • Redundant bincode::serialize(metadata) on the schema-less zenoh send path (perf) — genuine micro-opt, but it sits on the single hottest, most safety-commented send function; restructuring risks the carefully-ordered fallback logic for one avoided small-metadata serialize.
  • ros2-bridge per-element string/wstring size cap in sequence.rs (hardening) — the element-count cap already bounds growth and per-element impact is bounded by serde's lazy decode + buffer exhaustion; too weak to act on.
  • edit_distance allocates an O(n·m) matrix per candidate in TypeRegistry::suggest (efficiency) — cold path (only on an already-failed type lookup); deprioritized per "hot paths over cold".
  • Derive signal names from libc constants (raised by the /simplify altitude pass on fix(message): don't mislabel signal 23 as SIGURG on macOS/BSD daemon hosts #3110) — a deeper fix than the #[cfg] arms, and libc is already a dependency, but it broadens scope to all divergent signals and can't be cross-platform-compile-verified (const-alias / missing-const risks on macOS/Windows) in this environment; left as a follow-up. fix(message): don't mislabel signal 23 as SIGURG on macOS/BSD daemon hosts #3110's fix still corrects the actual mislabel.

Validation (once, on the combined diff of all 5 PRs)

  • cargo fmt --all -- --check — clean (toolchain 1.97.1, matching CI).
  • cargo clippy --all (excl. Python packages) -- -D warnings — clean.
  • cargo test --all (excl. Python packages, dora-examples, and dora-ros2-bridge*) — 39 test binaries pass. The only failures are 2 zenoh_schema_cache integration tests, both failing at Session::open() with tcp/[::]:0: Address family not supported — the sandbox has no IPv6. That test file is untouched by this diff, so the failure is environmental, not a regression.
  • /review and /simplify gates run on the combined diff; both surfaced findings were addressed (the disk-I/O panic-reset desync and the JSON first-column duplication) or documented as deferred above.
  • Environment notes: dora-ros2-bridge* was excluded from the test run because it needs a ROS2 distro (AMENT_PREFIX_PATH) and its example/test binaries exhausted the sandbox's fixed disk allowance during linking (ld SIGBUS); it is unrelated to this diff.

Each PR is a draft and clearly attributed to Claude (Claude Code). Please review carefully before merging — these are machine-generated.


Generated by Claude Code

Copy link
Copy Markdown
Collaborator Author

Reviewed the doctests against the actual implementations in libraries/core/src/types.rs — no issues found; all assertions match the real behavior:

  • types_match: identical-match, unparameterized wildcard, disjoint-key compatibility, shared-key conflict, and different-base mismatch all reflect the parse-and-compare logic exactly.
  • is_compatible: the Bytes sink, one-/two-hop builtin widening, same-base wildcard, and the 4-rule chain all correctly demonstrate the depth-3 BFS bound (the a→e case is unreachable because d is only dequeued at depth 3, where depth >= 3 skips expansion before reaching e).

The API paths (dora_core::types::{types_match, CompatibilityGraph, TypeRule}) resolve correctly, and the doctests genuinely exercise the conversions rather than just compiling. Documentation-only, no behavior change.


🤖 Automated review by Claude (Claude Code). This review was generated fully automatically with no human in the loop and has not been vetted by a maintainer — treat it as advisory.


Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants