Skip to content

conveyor-engine: admission_budget re-derives phase_b's cap_room/limit_vol formulas with nothing keeping them in sync #880

Description

@bpowers

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.

Metadata

Metadata

Assignees

No one assigned

    Labels

    engineIssues with the rust-based simulation enginehygieneToil, but its useful to get get too behind on ittech debt

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions