docs: thin the commentary back to what the code does not say

crates/xtce-codegen-e2e/tests/against_interpreter.rs

@@ -167,8 +167,6 @@ fn ctim_generated_matches_interpreted_on_every_packet() {
 /// IDEX: a binary field whose width comes from `PKT_LEN`, with two fields after it.
 ///
 /// This is the case that forced the cursor. Everything up to the blob is read at literal
-/// offsets; the blob and the two fields behind it are walked, because where they sit is a
-/// property of the packet.
 #[test]
 fn idex_generated_matches_interpreted_on_every_packet() {
     compare_with_interpreter!(
@@ -220,7 +218,6 @@ fn synthetic_packet(apid: u16, payload: usize) -> Vec<u8> {
 /// `APID_6`, `APID_10` and `APID_28` — never occur in it. Without this, compiled string
 /// decoding would have no comparison against the interpreter at all, which is precisely the
 /// kind of gap the differential suite exists to close. Synthetic packets are enough, because
-/// both decoders read the same bytes and any difference between them is real.
 #[test]
 fn ctim_string_containers_match_on_synthetic_packets() {
     let db = XtceDb::from_path(testdata("ctim/ctim_xtce_v1.xml")).expect("definition loads");
@@ -286,8 +283,6 @@ fn ctim_strings_are_borrowed_from_the_packet() {
 ///
 /// The packets are generated rather than fixed: bit patterns matter here — NaN payloads,
 /// subnormals, the sign bit of a 63-bit two's-complement field — and a handful of
-/// hand-written packets would test whichever ones happened to be written. A fixed seed keeps
-/// a failure reproducible.
 #[test]
 fn every_numeric_shape_matches_the_interpreter() {
     const BYTES: usize = 80;
@@ -332,8 +327,6 @@ fn every_numeric_shape_matches_the_interpreter() {
 /// has no answer outside its points. Agreement therefore has to include agreeing to fail, and
 /// on the same packets — which the macro, written when nothing could fail, does not express.
 ///
-/// `calibrators.xml` is deliberately built so both outcomes are common: two of its splines
-/// sit on a four-bit field whose range is wider than their points.
 #[test]
 fn calibration_matches_the_interpreter_bit_for_bit() {
     const BYTES: usize = 23;
@@ -426,7 +419,6 @@ fn calibration_matches_the_interpreter_bit_for_bit() {
 ///
 /// Without this, an emitter that sent both through one path would pass everything above —
 /// the two fields are compared against the interpreter separately, and a test that never
-/// feeds them the same number never notices they agree when they should not.
 #[test]
 fn the_integer_power_path_is_not_the_float_one() {
     // 2^27 + 1. Its cube needs 82 bits, so rounding it once is not the same as rounding the
@@ -461,8 +453,6 @@ fn the_integer_power_path_is_not_the_float_one() {
 /// A calibrator on an enumeration or a boolean is ignored, because the reference ignores it.
 ///
 /// XTCE looks both up from the *raw* value, and the interpreter returns before it consults a
-/// calibrator. `ENUM_CAL` and `BOOL_CAL` carry one anyway; a generator that helpfully applied
-/// it would turn `ARMED` into 1000.
 #[test]
 fn a_calibrator_on_an_enumeration_or_boolean_is_ignored() {
     let mut packet = vec![0u8; 23];
@@ -500,8 +490,6 @@ fn a_calibrator_on_an_enumeration_or_boolean_is_ignored() {
 ///
 /// The loop below has to tolerate failure on both sides, and for two different reasons: a
 /// packet may match nothing, or it may match two inheritors at once. `boolean_criteria.xml`
-/// gives Alpha and Beta overlapping branches so that the second case is common, because an
-/// OR is exactly what makes it easy to write by accident.
 #[test]
 fn a_boolean_expression_tree_matches_the_interpreter() {
     let db = XtceDb::from_path(testdata("boolean_criteria.xml")).expect("definition loads");
@@ -616,8 +604,6 @@ fn a_boolean_expression_tree_matches_the_interpreter() {
 /// reversed load; where they do not, the reversal has to happen after the read, and for a
 /// twelve-bit field it produces a value wider than the field it came from.
 ///
-/// One packet in four carries an APID the definition does not describe, so both
-/// implementations have to refuse the same packets as well as decode the same ones.
 #[test]
 fn byte_order_matches_the_interpreter() {
     let db = XtceDb::from_path(testdata("byte_order.xml")).expect("definition loads");
@@ -687,8 +673,6 @@ fn byte_order_matches_the_interpreter() {
 /// the file loads, and both of them read whatever it produced — so this test is not proving
 /// the semantics, which `crates/xtce-model/tests/arrays.rs` pins against the XTCE text. It is
 /// proving that reading 27 fields whose offsets came from an expansion is no different from
-/// reading 27 fields written out by hand, including the six nibbles that are not byte-aligned
-/// and the three floats behind them.
 #[test]
 fn arrays_match_the_interpreter() {
     const BYTES: usize = 39;
@@ -777,8 +761,6 @@ fn the_expanded_field_names_are_the_ones_xtce_describes() {
 /// Like arrays, this has no Python rung — the reference refuses aggregates too. And like
 /// arrays, the comparison cannot see a misreading the two implementations share, because both
 /// read the same expansion. It is here for what it *can* prove: that seventeen fields whose
-/// offsets came out of a nesting are read the same way as seventeen written out by hand,
-/// including the four-bit member that leaves everything after it off a byte boundary.
 #[test]
 fn aggregates_match_the_interpreter() {
     const BYTES: usize = 37;
@@ -817,7 +799,6 @@ fn aggregates_match_the_interpreter() {
 ///
 /// Checked against the XTCE text rather than against the interpreter, for the same reason as
 /// the array version: the interpreter derives its names from the same expansion, so a shared
-/// misreading would pass a comparison between them.
 #[test]
 fn the_nested_field_names_are_the_ones_xtce_describes() {
     let packet = vec![0u8; 37];
@@ -1127,8 +1108,6 @@ fn telecommands_match_the_interpreter() {
 /// Driven deliberately: the loop above proves they agree with the interpreter, and this proves
 /// what they agree *on*. `POWER = "ON"` and `POWER = "OFF"` are the same comparison against
 /// different labels, and a resolution that lost the distinction — took the first label, or the
-/// raw value of the literal, or matched everything — would still pass a test that only asked
-/// whether the two implementations said the same thing, if both were wrong the same way.
 #[test]
 fn an_assignment_on_an_enumeration_picks_the_command_by_its_label() {
     let mut packet = vec![0u8; 24];

crates/xtce-codegen/src/emit.rs

@@ -449,12 +449,6 @@ fn byte_slice(field: &Field, packet: &Ident) -> TokenStream {
 }
 
 /// The body of a container's `decode`.
-///
-/// Two shapes. When every width is fixed, the packet is narrowed to an array once and each
-/// field is a literal offset into it — no cursor exists at run time. When one field takes its
-/// width from the packet, everything before it still reads that way, and only the tail is
-/// walked with a cursor. The split is the honest one: the offsets after a data-dependent
-/// width are a property of the data, not of this generator.
 fn decode_body(plan: &ContainerPlan) -> TokenStream {
     let packet = ident("packet");
     let prefix_bits = if plan.is_dynamic() {
@@ -736,8 +730,6 @@ fn accessor(field: &Field) -> Option<TokenStream> {
 /// addition is neither associative nor commutative, so "the same arithmetic in a different
 /// order" is a different answer, and a calibrator is where that is easiest to get wrong.
 ///
-/// Two things carry that:
-///
 /// * **Terms are summed in document order**, not by Horner's method and not sorted.
 /// * **The integer and float paths are separate.** For an integral raw the power is computed
 ///   in `i128` and converted once, exactly as the reference does with its arbitrary-precision
@@ -1093,9 +1085,6 @@ const fn mask_for(width: u32) -> u64 {
 
 /// The expression that extracts `width` bits at `offset` as a `u64`.
 ///
-/// The byte span, shift and mask are all decided here, so the emitted code is a load, a
-/// shift and an `and` — no loop, no cursor, no branch.
-///
 /// The load uses the narrowest integer that spans the field rather than always padding to
 /// eight bytes. That is not cosmetic: a 1.6 MB mission database has nine thousand fields, and
 /// padding every one of them to an eight-element array literal turned the generated file into
@@ -1105,11 +1094,6 @@ const fn mask_for(width: u32) -> u64 {
 /// beyond the packet for a field that ends at its final byte, which on a fixed-size array is
 /// a compile error rather than something to discover later.
 /// A generated expression, and whether embedding it needs parentheses.
-///
-/// Rust binds `as` and `<<` tighter than `&`, so splicing `a & b` into `#expr as u32` would
-/// silently produce `a & (b as u32)`. Tracking atomicity means the parentheses go in exactly
-/// where they are needed — adding them everywhere would trip `unused_parens`, which a file
-/// meant for `include!` cannot switch off.
 #[derive(Clone)]
 struct Expr {
     tokens: TokenStream,
@@ -1832,9 +1816,6 @@ fn swap_helper() -> TokenStream {
 /// exponent in the bottom, neither biased, no implicit leading one, no infinities and no
 /// NaN. Line for line what `xtce_decode::decoder::mil_std_1750a` computes, which in turn is
 /// what the reference computes: `mantissa * 2 ** (exponent - 23)`.
-///
-/// Both fields are masked before the sign extension, so the shifts below have nothing above
-/// their width to discard and the masking and unmasked forms coincide.
 fn mil_helper() -> TokenStream {
     quote! {
         fn mil_std_1750a(bits: u64) -> f64 {
@@ -1877,13 +1858,6 @@ fn powi_helper() -> TokenStream {
 }
 
 /// Spline interpolation, line for line as `xtce_decode::calibrate::interpolate` does it.
-///
-/// The order and the extrapolation flag are passed in rather than specialised away, so that
-/// the two implementations can be read side by side; both are constants at every call site,
-/// so nothing survives optimisation.
-///
-/// Orders above one and an empty point list are refused when the code is generated, which is
-/// why the only failure left here is a query outside the points.
 fn spline_helpers() -> TokenStream {
     quote! {
         fn spline_line(x: f64, x0: f64, x1: f64, y0: f64, y1: f64) -> f64 {

crates/xtce-codegen/src/plan.rs

@@ -350,11 +350,6 @@ impl GuardTest {
 /// operator is therefore meaningful and every one is resolvable here: the enumeration is a
 /// finite list, so the answer for each entry is known when the code is generated, and what is
 /// left for run time is a membership test over the raw values.
-///
-/// Adjacent ranges are merged where the `<EnumerationList>` is in ascending order, which it
-/// almost always is, so an enumeration whose labels all satisfy the comparison becomes one
-/// range rather than two hundred. Out of order it simply does not merge: the set is the same
-/// either way, only longer to write.
 fn labels_satisfying(
     entries: &[(i128, i128, String)],
     operator: CompareOp,
@@ -488,10 +483,6 @@ const MAX_DEPTH: usize = 64;
 
 /// How far into the packet the layout has reached, while it is still knowable.
 ///
-/// Becomes `None` the moment a field's width comes from the data: from there on the offsets
-/// are whatever the packet says, and the generated decoder has to carry a cursor.
-/// `bytes` as exactly `bit_width.div_ceil(8)` bytes, right-aligned, leading bits cleared.
-///
 /// XTCE gives a fixed value as `hexBinary` and its width separately, and does not require the
 /// two to agree. Reading the bytes as one big-endian unsigned number and keeping the low
 /// `bit_width` bits of it is the only interpretation that handles both directions: a value

crates/xtce-codegen/tests/commands.rs

@@ -38,7 +38,6 @@ fn container<'p>(plan: &'p plan::Plan, name: &str) -> &'p plan::ContainerPlan {
         .unwrap_or_else(|| panic!("no container named {name}"))
 }
 
-/// Every fixed value reaches the plan, at the bit it occupies, inherited ones included.
 #[test]
 fn fixed_values_keep_their_place() {
     let plan = commands_plan();
@@ -78,8 +77,6 @@ fn fixed_values_keep_their_place() {
 ///
 /// XTCE requires both `binaryValue` and `sizeInBits` and does not require them to agree.
 /// `TRAILER` gives four bytes for a sixteen-bit field: read as one big-endian number,
-/// `DEADBEEF` in sixteen bits is `BEEF`, and an encoder that wrote `DEAD` — or all four bytes
-/// — would produce a packet nothing recognises.
 #[test]
 fn a_fixed_value_wider_than_its_field_is_truncated_from_the_left() {
     let plan = commands_plan();
@@ -104,8 +101,6 @@ fn a_fixed_value_wider_than_its_field_is_truncated_from_the_left() {
 /// A value narrower than its field is zero-extended, and spare leading bits are cleared.
 ///
 /// The other direction of the same rule. `SPARE` is four bits and its `binaryValue` is a
-/// whole byte, `0A`: the byte that reaches the plan has the high nibble cleared, so a caller
-/// writing it into a four-bit field cannot spill into whatever sits above it.
 #[test]
 fn a_fixed_value_narrower_than_its_bytes_has_its_leading_bits_cleared() {
     let plan = commands_plan();
@@ -119,7 +114,6 @@ fn a_fixed_value_narrower_than_its_bytes_has_its_leading_bits_cleared() {
     assert_eq!(spare.value, vec![0x0A]);
 }
 
-/// A container with no fixed values has none, rather than an empty one invented for it.
 #[test]
 fn a_telemetry_container_has_no_fixed_values() {
     let db = XtceDb::from_path(testdata("commands.xml")).expect("definition loads");
@@ -138,8 +132,6 @@ fn a_telemetry_container_has_no_fixed_values() {
 /// A criterion on an enumeration resolves to the raw values its labels cover.
 ///
 /// Not a string comparison at run time, and not a comparison against the literal read as a
-/// number either: the labels are a finite list, so which of them satisfy the operator is known
-/// when the code is generated, and what is left is a set of raw values.
 #[test]
 fn a_criterion_on_a_label_becomes_a_set_of_raw_values() {
     let plan = commands_plan();
@@ -188,8 +180,6 @@ fn collect(node: &plan::Node, out: &mut Vec<plan::Guard>) {
 
 /// A criterion comparing a *raw* enumeration value is an ordinary number comparison.
 ///
-/// `useCalibratedValue="false"` says to compare the bits, and the bits are a number. Only the
-/// engineering value is a label.
 #[test]
 fn a_raw_criterion_on_an_enumeration_stays_a_number() {
     let db = XtceDb::from_xml(

crates/xtce-codegen/tests/generated_decoder.rs

@@ -62,7 +62,6 @@ fn the_committed_decoder_is_what_the_generator_produces() {
     );
 }
 
-/// Both decoders, over every packet of the real stream, compared field by field.
 #[test]
 fn generated_matches_interpreted_on_every_packet() {
     let db = load();
@@ -200,8 +199,6 @@ fn a_packet_of_another_type_is_refused() {
 /// and its `<BooleanExpression>` compiles.
 ///
 /// So the assertion is inverted. If one of these ever stops compiling, that is either a
-/// regression or a definition that grew a construct outside the subset, and either way it
-/// should be noticed here rather than in whatever downstream build goes quiet.
 #[test]
 fn every_bundled_definition_compiles() {
     let directory = Path::new(env!("CARGO_MANIFEST_DIR")).join("../../testdata/spp");
@@ -292,8 +289,6 @@ fn planned_offsets_match_the_interpreters_offsets() {
 
 /// Constructs that must stay refused, each with the reason it cannot be compiled.
 ///
-/// Written inline because no bundled definition contains them, and a refusal path with no
-/// test is a refusal that can silently turn into a wrong answer.
 #[test]
 fn constructs_outside_the_compilable_subset_are_refused() {
     fn definition(types: &str, parameters: &str, entries: &str) -> String {
@@ -416,8 +411,6 @@ fn constructs_outside_the_compilable_subset_are_refused() {
 /// engineering value is a float, or a boolean, whose engineering value is 0 or 1 rather than
 /// its raw bits — comparing the raw bits would select a different container. Silently.
 ///
-/// The first of those became reachable the day calibrators started compiling: before then a
-/// calibrated field was refused outright, so a criterion could not name one.
 #[test]
 fn criteria_the_generator_would_evaluate_differently_are_refused() {
     fn definition(types: &str, parameters: &str, criteria: &str) -> String {

crates/xtce-decode/src/bits.rs

@@ -513,8 +513,6 @@ mod tests {
         /// Every width at every offset, against the one-bit-at-a-time oracle.
         ///
         /// The generator is written so that width 64 at a non-byte-aligned offset — the
-        /// nine-byte case — is actually reachable, rather than being buried under a uniform
-        /// distribution that would hit it once in a thousand runs.
         #[test]
         fn matches_naive_reference(
             data in proptest::collection::vec(proptest::num::u8::ANY, 1..24),
@@ -535,7 +533,6 @@ mod tests {
             }
         }
 
-        /// Wide fields specifically: nine-byte spans and everything around them.
         #[test]
         fn wide_reads_at_every_bit_offset(
             data in proptest::collection::vec(proptest::num::u8::ANY, 9..17),
@@ -548,7 +545,6 @@ mod tests {
             proptest::prop_assert_eq!(cursor.read_uint(width), Ok(want));
         }
 
-        /// Reading N bits as bytes and as an integer must agree for N <= 64.
         #[test]
         fn bytes_and_integers_agree(
             data in proptest::collection::vec(proptest::num::u8::ANY, 1..20),
@@ -578,8 +574,6 @@ mod tests {
 
         /// Sign extension at every width, against the arbitrary-precision oracle.
         ///
-        /// Width 63 is the interesting one and a uniform generator finds it once in sixty
-        /// runs, so widths are drawn across the whole range deliberately.
         #[test]
         fn twos_complement_matches_naive_reference(
             bits in proptest::num::u64::ANY,
@@ -610,7 +604,6 @@ mod tests {
             );
         }
 
-        /// Above the field's width the two part company, and the unmasked one is right.
         #[test]
         fn unmasked_sign_extension_keeps_the_high_bits(
             bits in proptest::num::u64::ANY,
@@ -629,7 +622,6 @@ mod tests {
             );
         }
 
-        /// A read never panics and never moves the cursor on failure.
         #[test]
         fn failures_leave_the_cursor_untouched(
             data in proptest::collection::vec(proptest::num::u8::ANY, 0..8),

crates/xtce-decode/src/decoder.rs

@@ -913,15 +913,6 @@ impl<'db> Decoder<'db> {
 }
 
 /// The largest number of entries any single decode path from `root` can visit.
-///
-/// A decode walks from the root down one chain of inheritors, decoding each container's own
-/// entry list and expanding any `<ContainerRefEntry>` inline. This is the deepest such walk,
-/// used to size a packet's storage once instead of regrowing it.
-///
-/// Container expansion is memoised, which is what actually bounds the work: the depth limit
-/// alone would not: a chain of forty containers each referencing the next twice is 2^40
-/// calls before the cap ever fires. Decoding has the same shape but is braked by running out
-/// of packet; this runs at construction time with no such brake.
 fn longest_path_entries(db: &XtceDb, root: ContainerId) -> usize {
     let mut expanded: Vec<Option<usize>> = vec![None; db.containers().len()];
     let mut deepest: Vec<Option<usize>> = vec![None; db.containers().len()];