Found by branch review of conveyor-engine before merge (the in-flight XMILE conveyor+queue implementation; specs docs/design/conveyors.md + docs/design/queues.md). Filed from a review report that exceeded its 15-finding cap.
Summary
admission_budget (src/simlin-engine/src/conveyor.rs:959-976) re-derives phase_b's cap_room/limit_vol formulas (conveyor.rs:848-862) verbatim, differing only in the subtracted conveyor volume (other_conv_vol vs conv_vol). The rustdoc at :939-941 promises the formulas stay identical, but nothing enforces it, so the queue-coupling admission budget can silently drift from what phase_b actually admits.
Cost
The cap_room/limit_vol derivation is duplicated across two functions that must stay identical for the queue coupling to be correct: admission_budget sizes what the queue serves, and phase_b then admits that queue volume unconditionally (via conv_inflows) and re-clamps only the equation inflows against the same formulas, with consume_inflow_budget bridging in_carry out-of-band. If a future edit changes one copy (a new freed-belt-room source, a different discrete in_carry rule) and not the other, the queue is sized against a stale budget while phase_b admits whatever the queue supplied -- over-capacity/over-limit belt material with no compile error.
Evidence
phase_b (conveyor.rs:851-862): let cap_room = if inp.capacity.is_infinite() { f64::INFINITY } else { (inp.capacity - contents_after - conv_vol).max(0.0) }; let limit_vol = if inp.in_limit.is_infinite() { f64::INFINITY } else if self.discrete { (inp.in_limit - self.in_carry).max(0.0) } else { inp.in_limit * dt };
admission_budget (conveyor.rs:961-976): let cap_room = if capacity.is_infinite() { f64::INFINITY } else { (capacity - contents_after - other_conv_vol).max(0.0) }; ... let limit_vol = if in_limit.is_infinite() { f64::INFINITY } else if self.discrete { (in_limit - self.in_carry).max(0.0) } else { in_limit * self.dt };
- rustdoc promise:
/// it serves. Uses the SAME cap_room/limit_vol formulas phase_b applies to (conveyor.rs:940)
phase_b admits conv_vol unconditionally: let admitted = conv_vol + eq_admitted; (conveyor.rs:885); the clamp applies only to the eq loop (:868-877)
- divergent inputs:
conv_vol built at conveyor_compile.rs:2043 (if inf.conveyor_driven || inf.queue_coupled {), other_conv_vol at :2109-2114 (.filter(|inf| inf.conveyor_driven)...sum())
- out-of-band seam:
conveyors[i].consume_inflow_budget(taken); (queue_compile.rs:1185)
Suggested direction
Extract one shared helper (e.g. fn admission_room(&self, phase_a, capacity, in_limit, conv_vol) -> (cap_room, limit_vol)) called by both, making the invariant structural. leaked/contents_after are local to the derivation and self.dt is shared, so the extraction is behavior-neutral.
Found by branch review of
conveyor-enginebefore merge (the in-flight XMILE conveyor+queue implementation; specsdocs/design/conveyors.md+docs/design/queues.md). Filed from a review report that exceeded its 15-finding cap.Summary
admission_budget(src/simlin-engine/src/conveyor.rs:959-976) re-derivesphase_b'scap_room/limit_volformulas (conveyor.rs:848-862) verbatim, differing only in the subtracted conveyor volume (other_conv_volvsconv_vol). The rustdoc at:939-941promises the formulas stay identical, but nothing enforces it, so the queue-coupling admission budget can silently drift from whatphase_bactually admits.Cost
The
cap_room/limit_volderivation is duplicated across two functions that must stay identical for the queue coupling to be correct:admission_budgetsizes what the queue serves, andphase_bthen admits that queue volume unconditionally (viaconv_inflows) and re-clamps only the equation inflows against the same formulas, withconsume_inflow_budgetbridgingin_carryout-of-band. If a future edit changes one copy (a new freed-belt-room source, a different discretein_carryrule) and not the other, the queue is sized against a stale budget whilephase_badmits whatever the queue supplied -- over-capacity/over-limit belt material with no compile error.Evidence
phase_b(conveyor.rs:851-862):let cap_room = if inp.capacity.is_infinite() { f64::INFINITY } else { (inp.capacity - contents_after - conv_vol).max(0.0) }; let limit_vol = if inp.in_limit.is_infinite() { f64::INFINITY } else if self.discrete { (inp.in_limit - self.in_carry).max(0.0) } else { inp.in_limit * dt };admission_budget(conveyor.rs:961-976):let cap_room = if capacity.is_infinite() { f64::INFINITY } else { (capacity - contents_after - other_conv_vol).max(0.0) }; ... let limit_vol = if in_limit.is_infinite() { f64::INFINITY } else if self.discrete { (in_limit - self.in_carry).max(0.0) } else { in_limit * self.dt };/// it serves. Uses the SAME cap_room/limit_vol formulas phase_b applies to(conveyor.rs:940)phase_badmitsconv_volunconditionally:let admitted = conv_vol + eq_admitted;(conveyor.rs:885); the clamp applies only to the eq loop (:868-877)conv_volbuilt atconveyor_compile.rs:2043(if inf.conveyor_driven || inf.queue_coupled {),other_conv_volat:2109-2114(.filter(|inf| inf.conveyor_driven)...sum())conveyors[i].consume_inflow_budget(taken);(queue_compile.rs:1185)Suggested direction
Extract one shared helper (e.g.
fn admission_room(&self, phase_a, capacity, in_limit, conv_vol) -> (cap_room, limit_vol)) called by both, making the invariant structural.leaked/contents_afterare local to the derivation andself.dtis shared, so the extraction is behavior-neutral.