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

crates/xtce-gs-cli/src/main.rs

@@ -65,12 +65,6 @@ fn main() -> ExitCode {
 }
 
 /// Prints a failure and everything under it, once each.
-///
-/// Every variant of every error enum in this workspace is `#[error("{0}")]` over a `#[from]`
-/// field, so the outermost message and its source's message are usually the same string. A
-/// chain printed without this prints it three times — `CliError`, `EngineError`, then
-/// `LinkError` — and an operator reading three identical lines learns to read none of them.
-/// Only a cause that *says something new* is printed.
 fn report(error: &dyn std::error::Error) {
     let mut previous = error.to_string();
     eprintln!("error: {previous}");

crates/xtce-gs-cli/src/run.rs

@@ -55,16 +55,8 @@ pub const PROGRESS_PACKETS: u64 = 1000;
 
 /// Lengths worth trying on a stream nobody has documented.
 ///
-/// 1115 is RS(255, 223) interleaved 5 deep, which is most of the missions in reach; 1279 is
-/// *that same link* measured marker to marker with the parity and the ASM counted, which is
-/// what an operator is usually told when they are told a number at all; 2048 is a common
-/// uncoded frame; 892 is RS(255, 223) interleaved 4 deep.
-///
-/// They are not all the same quantity, and that is the point of printing two columns per
-/// row: 1279 matching as a *stride* and 1115 matching as a *frame* are the same link, and an
-/// operator handed 1279 as a `--frame-length` gets a station that locks once and then finds
-/// nothing. [`advise`] is what turns a measured stride back into flags, and it is the line to
-/// read rather than the table.
+/// 1115 is RS(255,223) interleaved 5 deep; 1279 is that same link measured marker to marker,
+/// with the parity and the ASM counted, which is the number an operator is usually quoted.
 const FRAME_LENGTH_CANDIDATES: [u64; 4] = [1115, 1279, 2048, 892];
 
 /// Symbols in a Reed-Solomon codeword: CCSDS 131.0-B-5 section 4.
@@ -197,43 +189,16 @@ fn status_line(line: &mut String, snapshot: &StatsSnapshot, framed: bool) {
 }
 
 /// Prints the log lines `cursor` has not seen to stderr and moves it past them.
-///
-/// A cursor and never [`EventLog::clear`]: the log belongs to the session and the interface
-/// may be reading it too. [`EventLog::push`] bumps a collapsed line's count on every repeat,
-/// and the cursor is counted in pushes, so a line that keeps happening reprints with a rising
-/// count — which is what an operator watching a failing link wants, and what a printer that
-/// remembered the message instead of the position would suppress.
 fn print_events(log: &Mutex<EventLog>, cursor: &mut EventCursor, verbose: bool) {
     for line in unprinted_events(log, cursor, verbose) {
         eprintln!("{line}");
     }
 }
 
 /// How many pushes into an [`EventLog`] a printer has already considered.
-///
-/// A position and deliberately **not** a timestamp. [`Utc::now`] is `SystemTime::now`, whose
-/// granularity here is about a microsecond, so two events raised back to back usually carry
-/// the *same* stamp: a loop building 100 000 pairs of `Event::warning` and comparing their
-/// `time` found 93 258 of them tied. A cursor that skipped `time <= last` therefore dropped
-/// the second of such a pair, permanently, because the cursor only moves forward — and the
-/// line that costs an operator most is the last one, since [`headless`] prints once more
-/// after [`Session::shutdown`] and that is the line saying why the session stopped.
 type EventCursor = u64;
 
 /// The lines `cursor` has not seen yet, oldest first, moving it past them.
-///
-/// # Why a push count is an exact cursor
-///
-/// [`EventLog::total`] counts every push, repeats included, and [`EventLog::iter`] yields the
-/// distinct entries still held with how many pushes each stands for. So the entries account
-/// for the last `sum(count)` pushes, and the entry ending at push `n` can be named by `n`
-/// alone. That name survives eviction — dropping the oldest entry lowers `sum(count)` by
-/// exactly what it stood for, leaving every surviving `n` where it was — and only the *last*
-/// entry's count can grow, which is what makes a collapsed line reprint when, and only when,
-/// its count has risen.
-///
-/// The cursor moves past every entry it considers, printed or not, so an [`Severity::Info`]
-/// line held back without `--verbose` cannot make the warning behind it reprint.
 fn unprinted_events(log: &Mutex<EventLog>, cursor: &mut EventCursor, verbose: bool) -> Vec<String> {
     let mut lines: Vec<String> = Vec::new();
     let guard = log.lock().unwrap_or_else(PoisonError::into_inner);
@@ -273,14 +238,6 @@ fn unprinted_events(log: &Mutex<EventLog>, cursor: &mut EventCursor, verbose: bo
 /// default disposition kills the process outright, and a process killed by a signal runs no
 /// destructor: up to [`xtce_gs_engine::record::RECORD_BUFFER_BYTES`] of recording never
 /// reaches the file and [`xtce_gs_engine::record::Recorder`]'s `Drop` safety net never runs.
-///
-/// Measured with `xtce-gs run … --headless --record`, killed two seconds in: with ctrl-c
-/// alone handled, `kill -TERM` ended the process with status 143, the log stopped at
-/// `reading file://…` with no final counter line, and the recording ended at the last full
-/// buffer; with this, `kill -TERM` and `kill -INT` both exit 0, print the final line, and
-/// leave a recording exactly as long as the `bytes=` the station reports.
-///
-/// Windows has no SIGTERM and `ctrl_c` there already covers what the console sends.
 async fn stop_requested() -> std::io::Result<()> {
     #[cfg(unix)]
     {
@@ -536,13 +493,6 @@ async fn export_loop(
 }
 
 /// Whether a failure is the reader at the other end of the output going away.
-///
-/// `xtce-gs export … | head -1` is an ordinary way to look at an export — this module's
-/// header names redirection as what the subcommand is for — and Rust masks SIGPIPE, so a
-/// write to a pipe nobody is reading comes back as `EPIPE` instead of ending the process.
-/// Treating that as a failure gives exit 1 and `error: Broken pipe (os error 32)`, which
-/// aborts any `set -e` script that samples the header of a CSV. Nothing is lost that the
-/// operator asked for: the reader stopped listening first.
 fn is_broken_pipe(error: &EngineError) -> bool {
     matches!(error, EngineError::Io(io) if io.kind() == std::io::ErrorKind::BrokenPipe)
 }
@@ -907,11 +857,6 @@ fn report_frame_trial<W: Write>(
 /// whole number of 255-symbol codewords when the link is coded. So the block is the stride
 /// less the four marker octets, and a block that divides by 255 is Reed-Solomon with the
 /// quotient as the interleaving depth.
-///
-/// The parity cannot be recovered from the stride — E=8 and E=16 produce the same block
-/// length and differ only in how much of it is the frame — so both are offered. Guessing one
-/// would be guessing the frame length, and a frame length that is wrong by 80 octets per
-/// codeword is a synchroniser that never finds a second marker.
 fn advise(stride: u64) -> Vec<String> {
     let Some(block) = stride
         .checked_sub(CCSDS_ASM.len() as u64)
@@ -956,12 +901,6 @@ mod tests {
     use super::*;
 
     /// A definition with one container and one 32-bit parameter.
-    ///
-    /// Written from the test rather than read out of the sibling `xtce-rs` checkout the way
-    /// the engine's and the link crate's fixtures are: a test that skips depending on what
-    /// else the developer happens to have cloned is a test that stops being run, and nothing
-    /// below needs a mission — what is under test is where the export loop stops and what it
-    /// reports, not what the values mean.
     const MINIMAL_XTCE: &str = r#"<?xml version="1.0" encoding="UTF-8"?>
 <xtce:SpaceSystem name="TestSystem"
                   xmlns:xtce="http://www.omg.org/spec/XTCE/20180204">

crates/xtce-gs-engine/src/decode.rs

@@ -154,11 +154,6 @@ pub struct SequenceTracker {
     last: Vec<Option<u16>>,
     /// Per APID: how many gaps have been reported quietly since the last full line, and how
     /// many packets have arrived contiguously since the last gap.
-    ///
-    /// A dropout is one line and that is right. A link losing one packet in three is a
-    /// thousand lines a second, each naming a different sequence count — so `EventLog`'s
-    /// collapsing cannot fold them, and a bounded log fills with them while the one line
-    /// that says something else is evicted. See [`SequenceTracker::observe`].
     burst: Vec<Burst>,
     gaps: u64,
     missing: u64,
@@ -374,12 +369,6 @@ pub struct PacketDecoder {
     stats: Arc<LinkStats>,
     events: Vec<Event>,
     /// Containers already warned about undescribed trailing bits, indexed by container.
-    ///
-    /// A definition that does not describe a whole packet does not describe *any* packet of
-    /// that container, so the warning is one fact about the definition and not one fact per
-    /// packet. Without this, a 7 200-packet replay puts 7 200 identical lines on a log that
-    /// holds 2 048 — the event log collapses repeats, but only while nothing else is
-    /// interleaved, and on a live station something always is.
     warned: Vec<bool>,
 }
 
@@ -726,24 +715,12 @@ mod tests {
 
     /// Gaps and missing packets, derived from *which packets were removed*.
     ///
-    /// The point is that this shares no arithmetic with [`SequenceTracker::observe`]: no
-    /// modulus, no subtraction of one count from another, no reading of a sequence count at
-    /// all. It counts holes in a list. An earlier version of this helper retyped `observe`'s
-    /// own `(current + 16_384 - previous - 1) % 16_384`, which could only ever prove that the
-    /// same expression gives the same answer twice.
-    ///
     /// `apids[i]` is the APID of position `i` in the *complete* stream and `removed` lists
     /// the positions taken out of it. Each APID is counted on its own, because CCSDS
     /// 133.0-B-2 §4.1.3.4 gives each APID its own count. Within one APID's own sub-stream a
     /// maximal run of consecutive removed positions is one gap that says the run's length is
     /// missing — except a run touching the start or the end of that sub-stream, which has no
     /// surviving packet on one side and so nothing that could notice it.
-    ///
-    /// Exact only while a per-APID run is shorter than a whole counter's worth: at 16 383 the
-    /// tracker reports 16 382 and at 16 384 the repeated count is reported as nothing at all,
-    /// where this would still report the run. Both call sites remove one to three in a row,
-    /// which is what a link loses; a fixture that removed thousands would need the wrap rules
-    /// here, and would be back to retyping `observe`.
     fn expected_from_removals(apids: &[u16], removed: &[usize]) -> (u64, u64) {
         let mut kept_by_apid: BTreeMap<u16, Vec<bool>> = BTreeMap::new();
         for (index, apid) in apids.iter().enumerate() {

crates/xtce-gs-engine/src/error.rs

@@ -31,13 +31,6 @@ pub enum EngineError {
     Io(#[from] std::io::Error),
 
     /// The configuration cannot be carried out.
-    ///
-    /// A history depth of zero, a limits file whose JSON does not parse, a spacecraft time
-    /// parameter no definition declares. `serde_json::Error` is deliberately *not* a variant
-    /// of its own: the contract fixes this enum at five, and a parse failure is something the
-    /// operator wrote, which is what this variant means. The message carries the file, the
-    /// line and the column, because "invalid configuration" sends someone reading JSON by
-    /// eye.
     #[error("{0}")]
     Config(String),
 }

crates/xtce-gs-engine/src/lib.rs

@@ -25,45 +25,29 @@
 //! # }
 //! ```
 //!
-//! # The decision the rest follows from
-//!
-//! `Decoder::decode` returns values that borrow the packet buffer *and* the definition.
-//! Nothing with that type can sit in a history or cross a channel, so [`decode::project`] is
-//! where the borrow ends and [`xtce_gs_core::Value`] is what it ends into. After that point
+//! `Decoder::decode` returns values that borrow the packet buffer *and* the definition, and
+//! nothing with that type can sit in a history or cross a channel. [`decode::project`] is
+//! where the borrow ends and [`xtce_gs_core::Value`] is what it ends into; past that point
 //! nothing in this workspace names a lifetime.
 //!
-//! # What this crate refuses
-//!
-//! * **It does not stop on a bad packet.** A packet the definition does not describe is a
-//!   counter and a line in the log. A station that returned an error for one would stop on
-//!   the first, and every real downlink has them.
-//! * **It does not guess a time.** A configured spacecraft clock that cannot be resolved
-//!   fails [`Session::start`]; a packet that does not carry it falls back to ground receipt
-//!   and says which it used through [`xtce_gs_core::Batch::spacecraft`].
-//! * **It does not archive.** [`record::Recorder`] appends bytes and nothing else — no index,
-//!   no per-packet wrapper — so that a recording replays through the same source that made
-//!   it. A parameter archive is a different program.
-//! * **It does not draw.** Nothing here depends on egui, and the store is left in a state the
-//!   interface can read at any instant rather than one it has to be told about.
+//! A packet the definition does not describe is a counter and a log line, not an error —
+//! every real downlink has them. A recording is bytes and nothing else, so that it replays
+//! through the same source that wrote it.
 
 #![deny(missing_docs)]
 #![deny(clippy::unwrap_used, clippy::expect_used, clippy::panic)]
 #![warn(clippy::pedantic)]
 #![allow(clippy::module_name_repetitions)]
-// Spacecraft time is fixed-point arithmetic over fields of known width — a two-octet day
-// count, a three-octet binary fraction — and a plot axis is `f64` where a timestamp is `i64`.
-// Every `as` in this crate narrows or widens one of those with the width check that makes it
-// exact alongside; flagging them one at a time would mean an `#[allow]` on most of
-// `sctime.rs`.
+// Spacecraft time is fixed-point arithmetic over fields of known width, and every `as` here
+// carries the width check that makes it exact.
 #![allow(
     clippy::cast_possible_truncation,
     clippy::cast_possible_wrap,
     clippy::cast_precision_loss,
     clippy::cast_sign_loss,
     clippy::cast_lossless
 )]
-// Tests are allowed to assert loudly; the no-panic rule is about library code reached by a
-// live downlink, not about test setup.
+// The no-panic rule is about code a live downlink reaches, not about test setup.
 #![cfg_attr(
     test,
     allow(

crates/xtce-gs-engine/src/sctime.rs

@@ -338,7 +338,6 @@ mod tests {
 
     use super::*;
 
-    /// A CUC that is all zeroes is the epoch itself, whatever the widths.
     #[test]
     fn a_cuc_of_zero_is_the_ccsds_epoch_and_that_epoch_is_1958() {
         let time = cuc_from_bytes(&[0, 0, 0, 0, 0, 0], 4, 2, Utc::CCSDS_EPOCH).unwrap();