Design review report

[kdw503]

---

Design Review: RegisterWorkerShell

Likely Design Issues

1. getindex_t is scope-misaligned.
   This function slices a time axis from an AxisArray-style image — a general image utility with no intrinsic connection to worker lifecycle, monitoring, or
   shared-memory allocation. It reads like something extracted from concrete worker implementations and left here for lack of a better home. It's a thin wrapper
   over ImageAxes.timeaxis + view, which could reasonably live in ImageAxes itself. Its presence blurs the package's identity and forces every AbstractWorker
   package author to take a dependency on RegisterWorkerShell even for this general image-slicing operation.

2. workertid has no RemoteChannel forwarding method.
   Every other protocol function (init!, close!, worker, load_mm_package) has a corresponding rr::RemoteChannel forwarding method. workertid does not. A caller who
   writes workertid(rr) gets a MethodError rather than a forwarded result. This looks like an omission — workertid is listed alongside the other protocol functions
   in the module docstring.

3. workertid field convention is unenforced and gives poor errors.
   The requirement that all AbstractWorker subtypes must include a workertid field is stated only in a docstring. A subtype author who forgets gets FieldError: type
   MyWorker has no field workertid from inside RegisterWorkerShell.workertid — no indication that this is a contract violation. There's no trait check, validation
   function, or macro to catch it at definition time.

4. ArrayDecl{A, N} type parameter N is redundant and permits inconsistent states.
   A already encodes dimensionality via ndims(A), and the constructor enforces N = ndims(A). But ArrayDecl{Array{Float64,2}, 3}((2,3,4)) can be constructed directly
   — a well-typed but internally inconsistent object. If N were derived from A rather than free, this would be impossible. The only load-bearing use of N is
   arraysize::NTuple{N, Int}.

---

Design Questions

Q1. monitor silently skips undefined fields — was this intentional?
monitor(algorithm, (:fieldname,)) silently drops any symbol not defined as a field (isdefined(algorithm, f) || continue). This allows a field list to be a
superset of what any particular worker implements, but typos are invisible at construction time. Was the intent for the field list to be a soft suggestion
(absent fields silently ignored) or a strict contract (unknown fields warn or error)?

Q2. maybe_sharedarray passthrough for non-arrays — what is the intended call site?
maybe_sharedarray(obj, pid) returns obj unchanged for non-array objects, implying it's called in a generic loop over mixed-type fields. Is there such a loop in a
concrete algorithm package, or in RegisterWorkerShell itself? If this method exists to avoid dispatch at call sites, it's reasonable but worth documenting —
otherwise maybe_sharedarray(42) succeeding silently could mask caller mistakes.

Q3. No RemoteChannel forwarding for monitor/monitor! — by design?
Is this because monitor is always constructed on the driver side using local field values, while the worker receives mon by value? If yes, this is correct and a
brief comment would prevent future readers from wondering. If monitor! could ever be called on the worker side and results need propagating back, the absence
would be a latent bug.

Q4. Is "mm" in load_mm_package expected to be opaque to implementers?
"mm" stands for "mismatch module," a pipeline-internal concept not defined in this package. For an interface package whose users are concrete algorithm authors,
a name requiring out-of-band knowledge is a barrier. Is the audience narrow enough that "mm" is universally understood, or would a more descriptive name (e.g.,
load_device_package) be more self-documenting?

Q5. Is there a plan for an AbstractWorker compliance checker?
The interface requirements (implement worker, include workertid field) are documented but unverifiable at load time. Is a utility like
check_worker(::Type{<:AbstractWorker}) planned? This would give concrete algorithm authors fast feedback rather than runtime surprises.

---

Observations

• _getindex_t internal methods: The two underscore-prefixed dispatch branches are a clean optional-value dispatch pattern. Not exported, not public, not directly
  tested — appropriate for internals.
• Module docstring placement: The docstring is attached to the bare name RegisterWorkerShell at line 52 (after exports and type definitions). This is valid Julia
  and produces correct ?RegisterWorkerShell REPL output, but the conventional placement is immediately before or at the top of the module body.
• Vector{W} where {W<:AbstractWorker} in the multi-worker monitor: Excludes heterogeneous Vector{AbstractWorker} containers. Whether this matters depends on
  whether mixed-type worker vectors arise in practice.

---

Overall

RegisterWorkerShell is small and focused, and its core protocol design is sound. The lifecycle hooks (init!/worker/close!), the monitoring system
(monitor/monitor!), and the transparent RemoteChannel forwarding are all well-conceived. The monitoring system is particularly well-designed — the
creation/update split is principled, and the copyto! path in monitor! correctly handles the performance-sensitive case of large repeated arrays.

The main tension is the package's identity as a pure protocol interface vs. the presence of getindex_t, which is a general image utility with no protocol role.
The two highest-leverage changes would be: (1) relocating or re-examining getindex_t, and (2) adding a RemoteChannel forwarding method for workertid — which
appears to be a missing member of an otherwise complete family.