Source
PR #144 R1 review — bundled low-priority polish items.
Items
1. InvariantViolated.reason issue-ref fragility (code-reviewer SUG-2 conf 80)
The unreachable!() replacement at pak/mod.rs for the stream_entry_to dispatch arms uses:
reason: "stream_entry_to dispatch reached an unsupported \
CompressionMethod arm — early-reject at top of \
function was bypassed (see issue #138)"
The (see issue #138) reference is fragile: if #138 is closed and a new refactor reintroduces the bug, operators following the link land on the closed unreachable! removal ticket, not on whatever new issue tracks the regression.
Fix: drop the issue reference (the message text is self-explanatory and git-blame is the source of truth) OR rephrase to flag the staleness explicitly: "… was bypassed; #138 introduced the typed fallback — file a new issue if this fires".
File: crates/paksmith-core/src/container/pak/mod.rs (the InvariantViolated reason inside the match arm at the bottom of stream_entry_to).
2. BoundsUnit per-variant Display pin (pr-test sev 5)
BoundsUnit::Bytes => "bytes" and BoundsUnit::Items => "items" are wire-stable strings consumed by the same dashboards as the WireField/AllocationContext tokens. They're transitively exercised by IndexParseFault Display tests but with uneven coverage. PR #144 added per-variant pin tests for the two new enums; BoundsUnit should get the same treatment for consistency.
Fix: add bounds_unit_display_tokens_are_wire_stable modelled on the new wire_field_display_tokens_are_wire_stable (~5 lines).
File: crates/paksmith-core/src/error.rs (next to the WireField/AllocationContext pin tests).
Source
PR #144 R1 review — bundled low-priority polish items.
Items
1.
InvariantViolated.reasonissue-ref fragility (code-reviewer SUG-2 conf 80)The
unreachable!()replacement atpak/mod.rsfor thestream_entry_todispatch arms uses:The
(see issue #138)reference is fragile: if #138 is closed and a new refactor reintroduces the bug, operators following the link land on the closedunreachable!removal ticket, not on whatever new issue tracks the regression.Fix: drop the issue reference (the message text is self-explanatory and git-blame is the source of truth) OR rephrase to flag the staleness explicitly:
"… was bypassed; #138 introduced the typed fallback — file a new issue if this fires".File:
crates/paksmith-core/src/container/pak/mod.rs(the InvariantViolated reason inside the match arm at the bottom ofstream_entry_to).2.
BoundsUnitper-variant Display pin (pr-test sev 5)BoundsUnit::Bytes => "bytes"andBoundsUnit::Items => "items"are wire-stable strings consumed by the same dashboards as theWireField/AllocationContexttokens. They're transitively exercised by IndexParseFault Display tests but with uneven coverage. PR #144 added per-variant pin tests for the two new enums;BoundsUnitshould get the same treatment for consistency.Fix: add
bounds_unit_display_tokens_are_wire_stablemodelled on the newwire_field_display_tokens_are_wire_stable(~5 lines).File:
crates/paksmith-core/src/error.rs(next to the WireField/AllocationContext pin tests).