CLI 0.0.9 architectural review¶
Review date: 2026-09-21. Scope: the documentation proposal in
PR #193, against the
existing code at main fbb0887d0b41a290dc6b2cce9c3beb02884c42b4 and the selected
issue requirements. This is a design review, not a review of implemented features.
Verdict¶
Ready for implementation planning after corrections. An independent reviewer read AGENTS.md, the actual APIs and the design, raised the four findings below, then re-read the corrected design and confirmed all four were resolved at the architecture level, with no new blocking contradiction found.
The user approved the reviewed design on 2026-09-21. The subsequent implementation-plan review remains pending. Nothing was merged or published by this review.
Findings and resolutions¶
| Priority | Finding | Verified source evidence | Resolution in the design |
|---|---|---|---|
| P1 | A top-level target API cannot prevent nested ImageArt from re-detecting capabilities | Renderable receives only Console/ConsoleOptions; ImageArt's Renderable path invokes the legacy detector | An immutable per-Console protocol context reaches nested renderers; absent-context legacy behaviour is preserved. The ext serializer filters hyperlinks/control segments explicitly. Tests cover nested Panel/Layout art and contradictory environment values. |
| P1 | Preferred/fixed allocations can exceed the viewport even when minima fit | Core ratio_resolve can return requested fixed sizes/minima beyond the available extent | The ext bounded allocator shrinks preferred slack first, then relaxes impossible minima; explicit rounding, fixed-size semantics, capped flex padding and numeric examples make every case deterministic. Core ratio behaviour stays unchanged. |
| P1 | Live content taller than the terminal invalidates cursor geometry | LiveRender currently retains unclipped line height and defers vertical-overflow cropping | The coordinator clips to a bounded viewport, reserves an insertion row and guard column, defines zero/one-row behaviour, coordinates ordinary writes and invalidates geometry on resize. PTY/virtual-screen tests must verify actual visible state. |
| P2 | Grayscale source processing can leave coloured contain borders | ImageArt creates the contain canvas from the original background after processing the source | Grayscale transforms the effective background too; contain padding reuses it. Test transparent pixels and non-square content against a saturated background. |
Source files inspected: crates/rich/src/protocol.rs, console.rs, ratio.rs, live_render.rs and style.rs; crates/rich-art/src/image_art.rs; and the CLI batch planner. The ownership and package sequence were checked against AGENTS.md and workspace/internal Cargo requirements.
Additional clarifications¶
- Zero-sized explicit targets return an empty render rather than entering legacy one-cell-minimum paths.
- The Live print facade does not implicitly intercept Console::print; callers use the coordinator's handle and do not share its writer with another Live loop.
- Batch template tokens, escaping, index origin and extension ownership are explicit. New directory-preserving dry runs can preview missing parents without creating them, while legacy flat-mode behaviour remains unchanged.
- The original filesystem-race limitation is superseded by hardening #196: parent-owned directory handles, private worker staging, exclusive creation and replacement of output entries. See the CLI batch contract for the exact directory-object authority boundary and remaining limits.
- Geometry-only image transforms preserve RGBA where other preprocessing is off.
Verification and remaining risk¶
The change contains documentation only. Formatting, Clippy and the unchanged baseline Rust suite are rerun before committing. The suite is run with the session's ambient NO_COLOR removed because colour assertions require the normal test environment. No product test was weakened or source modified.
Live resize, terminal reflow and log insertion near the bottom edge are the main implementation feasibility gate. The tests must model cursor position, wrapping, scrolling and visible cells; finding valid escape sequences in output alone is insufficient. Snapshot fixtures for new extension behaviour must not replace or silently update the upstream parity goldens.
The revised design proposes core 0.0.5, ext 0.0.7, art 0.0.7 and CLI 0.0.9. These remain proposed package changes; this documentation update does not change current manifests or authorize tags.
Implementation evidence — 2026-09-21¶
The preceding review is architectural and does not stand in for code review.
Native implementation now has focused regressions per subsystem and combined
acceptance in expanded_release.rs. Default and all-feature suites, core goldens,
Clippy, defaultless builds and Rust 1.90 passed locally. Real PTY Live tests cover
resize, log retention and cursor cleanup. The docs build passes strict MkDocs.
Independent whole-branch implementation review follows the final release cohort
update; its findings and disposition will be recorded separately.
Independent implementation review and fixes¶
A fresh-context reviewer evaluated main fbb0887 through implementation f297f12,
using the seven plans' review focus and an isolated reproduction project. The
review found no Critical issues, six Important defects and one Minor issue. Its
verdict was not ready at the reviewed commit; this is not presented as approval
of later commits or as a second review.
| Finding | Fix and independent regression expectation |
|---|---|
| Cross-axis constraints ignored | Resolve both child dimensions; fixed-width child in vertical split renders ABC, fixed-height child in horizontal split renders only A |
| Nested intrinsic sizing drops content / clips wrapped height | Measure descendants independent of opt-in flags; allocate widths before measuring heights; ABCDEF survives nested sizing and wraps into two rows at width3 |
| Live ordinary writes leak disabled links | Share row normalization with regions; preserve ANSI color with no OSC8 |
| Live narrow pipe writes lose content | Use full noninteractive width and LF; width1 abc is exactly a\nb\nc\n |
| Combining-only span marks following character | Expand to intersecting grapheme boundaries; accent in éX marks é |
| Auto images ignore ASCII-only target | Explicit-context Auto selects ASCII; explicitly requested glyph modes remain caller opt-ins |
| Doctor provenance labels overrides/fallbacks detected | Preserve configured color policy and identify fallback dimensions |
The executor reproduced the failures before applying fixes and reran focused regressions, the Live PTY check, mandatory global gates and affected feature checks. The evidence file records final outcomes. No second independent review is claimed.
Decisions on behavior the reviewer set aside¶
- Arbitrary Console/raw stdout interception remains excluded: active Live writes must use the coordinator handle.
- Writer/interactivity replacement remains finish/new, as approved.
- Race-free concurrent filesystem replacement remains excluded; checked preflight and worker revalidation are the selected contract.
- Consumed generic streams retain one capture pass; decoded images rerender.
- Collision skip remains excluded; keep error/suffix/overwrite.
- Full Live parity, reflection, broader palettes/transforms and future protocols remain in their open roadmap issues.
- Third-party renderables that probe ambient state are outside built-in determinism.
- Negative/fallible target dimensions remain outside the infallible usize API.
These are explicit retained boundaries, not silently dropped findings. The PTY harness exercises transport and a bounded screen model; it is not proof of every terminal emulator's resize/reflow behavior.
Filesystem hardening follow-up (#196)¶
The user explicitly requested implementation in this release, superseding the original deferred race limitation. Batch workers now render to private staging; the parent publishes through retained directory handles. Descendant components are opened without following links; new files use exclusive creation, overwrite replaces entries, and missing parents are created through directory handles.
A fresh independent review found two Important issues: replacement could widen private-file permissions, and copying could delay cancellation. Regressions first failed for both. Replacement temporaries are now restrictive before any data is written (Unix mode 0600 at creation; Windows exclusive handle plus protected owner-only DACL). Copying checks cancellation between 64 KiB chunks and before replacement. Final independent review found no remaining blockers within the stated threat model and reran all 12 focused Unix tests successfully. Windows code cross-checks locally; real Windows ACL/junction/rename tests run in CI.
Local verification: 655 default tests, default and lean CLI all-target Clippy, Rust 1.90 all-feature checks, strict MkDocs, and real batch signal/progress checks pass. This review covers the hardening delta, not a second approval of every original expanded-release subsystem. No merge or publication was performed.