# Source code review checklist Software Level: **DAL-C** (DO-178C) Reviewer: author — **review without independence**, which DO-178C permits at Software Level C. Scope: `src/buffer/**`, `src/components/buffer.rs`, `src/config.rs`, `src/app.rs`, `src/cli.rs`, `src/action.rs` Derived from DO-178C Table A-5 (verification of the source code), reduced to the objectives that apply at DAL-C. --- ## Compliance and traceability | # | Check | Result | |---|---|---| | 1 | Source complies with the low-level requirements | **Pass** | | 2 | Source is traceable to the LLRs | **Pass** — 98/98 tagged `// MJB-LLR-nnn`, verified by `grep`, not inspection | | 3 | Source complies with the software architecture | **Pass** — buffer core has no `ratatui` dependency and is testable headless | | 4 | Source is verifiable | **Pass** — 96.21% statement coverage of `src/buffer/**` | | 5 | Source conforms to a coding standard | **Pass** — `cargo clippy --all-targets -- -D warnings` is clean | | 6 | Source is accurate and consistent | **Pass** — see the defects found, below | Reproduce objectives 2, 4 and 5: ```bash cargo clippy --all-targets -- -D warnings # clean cargo test # 294 passing cargo llvm-cov --summary-only test ``` --- ## No dead code | # | Check | Result | |---|---|---| | 7 | Removed widgets leave no unreachable code | **Pass** | | 8 | No `allow(dead_code)` masks anything | **Pass** | | 9 | Removed dependencies are gone from the graph | **Pass** | Evidence: ```bash # The widget modules are deleted, not merely unregistered. ls src/components/ # buffer.rs only grep -rni "FpsCounter\|mod fps\|mod home" src/ # only a comment in app.rs # No dead-code suppression anywhere in the crate. grep -rn "allow(dead_code)" src/ # nothing # The chord-timeout state is gone, not just unused. grep -rn "last_tick_key_events" src/ # nothing # JSON is out of the dependency graph, not merely out of the call path. cargo tree | grep -iE "json|yaml|ini\b" # nothing ``` Three items of inherited dead code were removed during review, beyond the two widgets the task named: - `Action::Help` — declared by the application template, never produced or handled. - `Command::leaves_insert_mode` — written by me during implementation, then never called. Caught because `buffer/command.rs` showed 0% coverage. - `#![allow(dead_code)]` in `src/tui.rs` — verified to mask nothing before removal, by deleting it and re-running clippy. --- ## Robustness and error handling | # | Check | Result | |---|---|---| | 10 | No panic on malformed input | **Pass** — invalid UTF-8 is a reported error (MJB-LLR-118); a declared non-UTF-8 encoding substitutes U+FFFD (MJB-LLR-113) | | 11 | No panic on malformed configuration | **Pass** — a bad key sequence is a recoverable error naming itself (MJB-LLR-183) | | 12 | No panic on a failed file operation | **Pass** — every `save` path returns `Result` | | 13 | Boundary conditions handled | **Pass** — empty buffer, no trailing newline, zero-height viewport, scrolloff exceeding the viewport, undo past history start | | 14 | Integer overflow/underflow guarded | **Pass** — `saturating_*` throughout `view.rs` and `movement.rs` | `unwrap`/`expect`/`panic!` outside test code is confined to two files, neither in the buffer core: | File | Count | Assessment | |---|---|---| | `src/errors.rs` | 1 | Inside the panic hook itself | | `src/tui.rs` | 2 | Template code in `Drop`; pre-existing | `src/buffer/**` contains **zero** panicking calls outside `#[cfg(test)]`. The one genuinely new failure mode created by this change — a byte offset landing inside a character, which makes ropey panic — is guarded in two places: `Range::clamped` (MJB-LLR-011) snaps every externally supplied offset onto a char boundary, and `ChangeSet::apply` (MJB-LLR-044) validates every operation boundary *before* mutating, so a rejected change leaves the rope untouched rather than half-applied. --- ## Defects found and fixed during review Eight defects were found by review and test activity rather than by writing the code. Each records how it was found, because that tells the next person which verification activities are actually working (docs/process.md §6, Rule 4). 1. **Every shifted key binding was shadowed by its lowercase twin.** `parse_key_event` lowercased the entire key string, so `""` and `""` both parsed to `KeyCode::Char('a')`. `A`, `I`, `O`, `U`, `W`, `B` and `E` were all unreachable, and whichever binding won the hash map ran instead — so `a` silently invoked `InsertAtLineEnd`. Found by seven integration tests failing at once. Fixed by preserving the final token's case and inferring `SHIFT` from an uppercase letter, matching what crossterm reports. 2. **UTF-16 files would have been corrupted on save.** `encoding_rs::Encoding::encode` refuses to encode to UTF-16 and silently substitutes UTF-8, so a UTF-16 document would have been written as UTF-8 bytes beneath a UTF-16 BOM. Invisible to inspection; found only by a byte-comparing round-trip test. Fixed by encoding UTF-16 directly from `str::encode_utf16`. Recorded as **MJB-DR-007**. 3. **`o` on a file with no trailing newline put the cursor in the wrong place.** `open_below` anchored to the next line's start, which does not exist on a final unterminated line. Found by clippy's `if_same_then_else` flagging the suspicious `if above { at } else { at }`, then confirmed by a robustness test. Fixed by anchoring to the line's content end. 4. **`a` appended in the wrong position.** Helix's normal-mode cursor is a one-grapheme range, so `a` lands past that grapheme; mojibake's empty range has `to() == from()`. Fixed by stepping forward one grapheme when the range is empty. A later quality pass over the same code found four more. Note that three were in paths no test covered and one was in inherited template code that only became reachable when the buffer started consuming `[styles]`. 5. **Counted word motion was broken.** Each iteration of the count loop re-derived its start from the partial range's *cursor*, which for a forward range is one grapheme behind the head. `2w` on `"aaa bbb ccc"` therefore returned `Range(3, 4)` — anchor in the wrong place, head stalled inside the first gap — instead of `Range(0, 8)`. Found by restructuring `word_move` into three named functions and checking each against a worked example; no test covered a count greater than one on a word motion. Fixed by holding the anchor fixed and advancing only the head. Regression tests `mjb_llr_065_counted_next_word_start_advances_once_per_count` and siblings. 6. **Three panics reachable from user configuration.** `parse_color` indexed `rgb` operands unchecked and added into `u8` unguarded, so `"rgb"`, `"rgb1"`, `"gray99"` and `"rgb999"` in a `[styles]` block aborted the editor at startup — a direct violation of MJB-HLR-018. Found by running clippy with `-W clippy::pedantic` and following up the `cast_possible_truncation` hits by hand. Fixed by bounds-checked parsing and clamping to the actual xterm palette ranges. Regression test `mjb_llr_183_malformed_colours_never_panic`. 7. **Bright colours were indistinguishable from their base colours.** The template wrote `c.wrapping_shl(8)`, which on a `u8` masks the shift to `8 % 8 == 0` and returns the value unchanged. Found while fixing defect 6. Fixed to the ANSI convention of base + 8, saturating. 8. **The modified flag stayed set after undoing back to the saved state.** A latched `bool` cannot distinguish "edited" from "edited and reverted", so a buffer byte-identical to its file still refused `:q`. Found by review of the flag's lifecycle rather than by a test. Replaced with a comparison against the history revision the file was written at. The first attempt at that fix was itself wrong and worth recording: using the history *cursor* as the revision means saving at depth 3, undoing, then making a **different** edit returns to depth 3 while the content differs — reporting clean when it is not. Caught by reasoning through the case before committing, and fixed by giving each history entry a unique, never-reused id. Regression test `mjb_llr_116_revision_is_not_stack_depth`. --- ## Architecture | # | Check | Result | |---|---|---| | 15 | The pre-release rope dependency is confined | **Pass** — `ropey::Rope` is owned only by `buffer/document.rs`; other modules take `RopeSlice` parameters (MJB-DR-006) | | 16 | The buffer core is independent of the UI | **Pass** — no `ratatui` import under `src/buffer/` | | 17 | Pagination is structural, not an optimisation | **Pass** — `View::visible_lines` is the only path to line content, and is bounded by viewport height | Objective 17 is the one most worth stating plainly: MJB-HLR-012 is satisfied because there is no code path that walks the document, not because a fast path was added. `mjb_llr_200_renders_only_the_visible_window` checks the observable consequence, and `mjb_llr_092_scrolling_a_large_file_stays_responsive` checks that 500 half-page scrolls over a 200,000-line file stay well inside a time budget a full scan could not meet. --- ## Summary Code review **complete**. Eight defects found and fixed; three items of dead code removed; no `allow(dead_code)` remains; clippy clean at `-D warnings`; 294 tests passing; 96.21% statement coverage of the buffer core. Outstanding items are the six open derived requirements in `docs/requirements/derived.md`, which need a safety-assessment judgement rather than a code change. MJB-DR-001 is resolved: UTF-8 is now validated strictly and malformed input is refused, so the lossy-save hazard it flagged is gone.