API review: widen annotations, rename prepare_mm_package, expose parallel keyword, bump to v1.0.1

API_REVIEW_PLAN.md

@@ -0,0 +1,128 @@
+# API Review Plan
+<!-- Auto-generated by /review-api. Edit freely, but preserve chunk IDs and status values. -->
+
+## Metadata
+- **Kind**: `api`
+- **Package**: RegisterDriver
+- **Source review date**: 2026-05-14
+- **Current version**: 1.0.0
+
+## Stated values
+모든 findings를 한 번에 적용한다. deprecation shim은 제공하지 않는다 — clean break. `mm_package_loader` → `load_mm_package` rename은 기술적으로 breaking이지만, 사용자의 판단으로 v1.0.1로 bump한다 (내부/소규모 사용자 환경). `driver`의 threading threshold는 `parallel` 키워드로 노출하되 기본값은 현재 동작(`length(algorithms) > 2`)을 보존해 기존 호출자에게 동작 변화가 없도록 한다.
+
+## Release strategy
+- **Pre-breaking-release**: `no`
+- **Inter-cluster releases**: `n/a`
+
+## Baseline
+- Tests pass on the starting commit: **9 pass, 1 fail** — pre-existing flaky test at `test/runtests.jl:88` (`@test length(indx) == length(tids) && all(indx .> 0)`). The test asserts that every thread ID returned by `threadids()` is actually used by `@threads :dynamic` worker tasks; the scheduler does not guarantee this, so the test is intermittently flaky. Not caused by any chunk in this plan; flagged in Open Questions.
+- `Test.detect_ambiguities` count: **0**
+- Working tree clean: yes for tracked files. Untracked: `API_REVIEW_PLAN.md` (this plan), `.claude/review_design_report.txt` (from earlier design review session).
+
+## Decisions
+<!-- Answers to `decide` chunks land here, with the chunk ID. -->
+- **CHUNK-004 rename target**: the originally chosen `load_mm_package` collides
+  with the imported `RegisterWorkerShell.load_mm_package` (line 20 of
+  `src/RegisterDriver.jl`), which is the very symbol the wrapper delegates to.
+  Renaming to `load_mm_package` would either shadow the import or require
+  qualifying every internal call. User chose a non-colliding verb-first name
+  `prepare_mm_package` instead, which preserves the import unchanged and reads
+  naturally for the wrapper's role (load/prepare the device-specific mismatch
+  package before `driver`).
+
+## Chunks
+
+### CHUNK-001: preflight
+- **Kind**: `preflight`
+- **Originating finding**: n/a
+- **Cluster**: none
+- **Breaking**: no
+- **Description**: Establish baseline — run full test suite, record `Test.detect_ambiguities` count, confirm clean working tree.
+- **Depends on**: none
+- **Verification**: `Pkg.test()`, `Test.detect_ambiguities(RegisterDriver)`
+- **Status**: `complete`
+- **Notes**: Baseline established 2026-05-14. 9/10 tests pass; one pre-existing flaky test at `test/runtests.jl:88` related to `threadids()` round-trip through `@threads :dynamic`. Ambiguities: 0. Julia 1.12.6, RegisterDriver v1.0.0.
+
+### CHUNK-002: widen-vector-annotations
+- **Kind**: `implement`
+- **Originating finding**: Tier 2-A — `Vector` → `AbstractVector` in `driver` (line 64) and `mm_package_loader` (line 238)
+- **Cluster**: annotation-widening
+- **Breaking**: no
+- **Description**: Change `algorithms::Vector` and `mon::Vector` in `driver` method 1 (line 64) to `AbstractVector`. Change `algorithms::Vector{W} where {W<:AbstractWorker}` in `mm_package_loader` method 1 (line 238) to `AbstractVector{<:AbstractWorker}`. All existing callers passing plain `Vector` continue to match.
+- **Depends on**: CHUNK-001
+- **Verification**: existing tests still pass; add a test passing a view or SubArray if feasible
+- **Status**: `complete`
+- **Notes**: Changed `algorithms::Vector` → `AbstractVector` and `mon::Vector` → `AbstractVector` on line 64; changed `mm_package_loader(algorithms::Vector{W}) where {W<:AbstractWorker}` → `mm_package_loader(algorithms::AbstractVector{<:AbstractWorker})` on line 238. Updated matching docstring lines. No new ambiguities. Tests: 9 pass / 1 pre-existing flake (unchanged from baseline).
+
+### CHUNK-003: widen-dict-annotations
+- **Kind**: `implement`
+- **Originating finding**: Tier 2-B — `Dict` → `AbstractDict` in `driver` methods 2 and 3 (lines 151, 171)
+- **Cluster**: annotation-widening
+- **Breaking**: no
+- **Description**: Change `mon::Dict` to `mon::AbstractDict` in `driver` method 2 (line 151) and method 3 (line 171). Allows `OrderedDict`, `IdDict`, and any other `AbstractDict` subtype. All existing callers passing a plain `Dict` continue to match.
+- **Depends on**: CHUNK-001
+- **Verification**: existing tests still pass
+- **Status**: `complete`
+- **Notes**: Changed `mon::Dict` → `mon::AbstractDict` on lines 151 and 171. Updated matching docstring. 0 new ambiguities. Tests: 9 pass / 1 pre-existing flake (unchanged).
+
+### CHUNK-004: rename-mm-package-loader
+- **Kind**: `implement`
+- **Originating finding**: Tier 1-A — `mm_package_loader` is a noun compound; Julia convention is verb-first (`load_mm_package`)
+- **Cluster**: none
+- **Breaking**: yes
+- **Description**: Rename exported function `mm_package_loader` → `prepare_mm_package` in both methods and update the export list. No deprecation shim. Update all internal call sites (if any), docstrings, and tests.
+- **Depends on**: CHUNK-001
+- **Verification**: tests pass; grep confirms no remaining references to `mm_package_loader`
+- **Status**: `complete`
+- **Notes**: Final name is `prepare_mm_package`, not the originally proposed `load_mm_package` — see Decisions (CHUNK-004 rename target). Updated `src/RegisterDriver.jl` (module-level docstring xref line 7, export list line 27, signatures in docstring lines 226–227, and both method definitions lines 238–239). Updated `README.md` line 90, `docs/src/index.md` line 91, and `docs/src/api.md` line 12. No internal call sites elsewhere; tests have no references to the old name. Grep confirms zero remaining references to `mm_package_loader` in source/docs. Test suite: 9 pass / 1 pre-existing flake (unchanged from baseline); Doctests, Aqua, and ExplicitImports all green. 0 ambiguities (unchanged).
+
+### CHUNK-005: expose-parallel-keyword
+- **Kind**: `implement`
+- **Originating finding**: Tier 3-A — threading threshold `length(algorithms) > 2` is a hard-coded hidden policy
+- **Cluster**: none
+- **Breaking**: no
+- **Description**: Add `parallel::Bool = length(algorithms) > 2` keyword argument to `driver` method 1 (line 64). The default preserves current behaviour exactly. When `parallel=true`, use `@threads :dynamic`; when `false`, use sequential iteration. Update docstring and tests.
+- **Depends on**: CHUNK-001
+- **Verification**: add tests for `parallel=true` and `parallel=false` with 1- and 3-worker inputs; existing tests still pass
+- **Status**: `complete`
+- **Notes**: Added `parallel::Bool = length(algorithms) > 2` keyword to `driver` method 1; renamed local `usethreads` → `parallel`. Default preserves prior behaviour (the prior `usethreads = nummon > 2` is equivalent because `nummon == nalgs`). Docstring updated. Added "parallel keyword" testset (4 tests): 1-worker `parallel=false`, 3-worker `parallel=false` (forces sequential), N-worker `parallel=true` (one alg per `threadids()`, mirroring the existing multi-thread test pattern), and the default-keyword call. Verified all 4 pass when invoked directly via MCP. The intended 1-worker `parallel=true` case was dropped — it exposes an existing design property of `driver`: the threaded loop only runs work on threads matching `workertid`, so a single worker pinned to one tid can't observe all `@threads :dynamic` iterations and produces an empty file. Noted in Open Questions. `Pkg.test()` shows the same baseline result as before this chunk (9 pass / 1 pre-existing flake at runtests.jl:88; runtests.jl is short-circuited by that flake before later testsets emit summaries — also pre-existing). Doctests, Aqua, ExplicitImports green. 0 ambiguities (unchanged).
+
+### CHUNK-006: version-bump
+- **Kind**: `version-bump`
+- **Originating finding**: n/a
+- **Cluster**: none
+- **Breaking**: yes
+- **Description**: Bump version from 1.0.0 → 1.0.1 per user's stated preference. Note: CHUNK-004 is technically a breaking change under semver (would normally warrant 2.0.0), but the author has deliberately chosen 1.0.1 given the small user base. Update `Project.toml` and CHANGELOG (if present).
+- **Depends on**: CHUNK-002, CHUNK-003, CHUNK-004, CHUNK-005
+- **Verification**: full test suite green; `Project.toml` shows 1.0.1
+- **Status**: `complete`
+- **Notes**: Bumped `Project.toml` `version` from `1.0.0` → `1.0.1`. No `CHANGELOG.md` exists; none created. Test suite identical to baseline (same pre-existing flake at `runtests.jl:88`; Doctests / Aqua / ExplicitImports green). Pkg.test report confirms `RegisterDriver v1.0.1`. NOTE for the user: `Project.toml` had pre-existing unstaged `[compat]` tightening in the working tree at session start (RegisterCore and RegisterWorkerShell narrowed from `"0.2, 1"` → `"1"`). Those edits are unrelated to CHUNK-006 but live in the same file. Decide before commit whether to bundle them with the version bump (sensible, since the package is already on 1.x) or split them out — `git restore --staged Project.toml` then re-stage selectively if you want them split.
+
+## Dropped findings
+<!-- Items the user chose not to act on, with one-line reasons. -->
+- **2a Dimension arguments**: not applicable — no dimension-taking functions in this package.
+- **2b Data-first ordering**: `driver`'s `outfile`-first signature follows the `write(io, data)` Base IO-first convention; no change needed.
+- **2c In-place/out-of-place pairing**: no asymmetric pairs in the public API.
+- **2d Boolean/integer flag arguments**: none present.
+- **2e Reduction `init`**: not applicable.
+- **2f Sorting/ordering**: not applicable.
+- **2g Output allocation**: file-based `driver` writes to disk; in-memory form returns a `Dict`; no hot-loop allocation concern.
+- **2h `do`-block compatibility**: no callable arguments in the public API.
+- **2i Keyword passthrough**: no wrapper functions with fixed keyword subsets.
+
+## Session Log
+> **Session 2026-05-14**: Completed CHUNK-001 (preflight). Established baseline: 9/10 tests pass, 0 ambiguities. The single failing test (`test/runtests.jl:88`) is a pre-existing flake in the `threadids()`/`@threads :dynamic` round-trip — independent of this plan but worth keeping in mind when CHUNK-005 (`parallel` keyword) adds related tests. Next up: CHUNK-002 (widen-vector-annotations).
+
+> **Session 2026-05-14**: Completed CHUNK-002 (widen-vector-annotations). Widened `Vector` → `AbstractVector` on `driver` method 1 (line 64, both `algorithms` and `mon` params) and on `mm_package_loader` method 1 (line 238, removing the `{W<:AbstractWorker}` type parameter in favor of `AbstractVector{<:AbstractWorker}`). Updated docstrings to match. 0 new ambiguities; 9 pass / 1 pre-existing flake (unchanged). Next up: CHUNK-003 (widen-dict-annotations).
+
+> **Session 2026-05-14**: Completed CHUNK-003 (widen-dict-annotations). Changed `mon::Dict` → `mon::AbstractDict` on `driver` method 2 (line 151) and method 3 (line 171). Updated docstring. `annotation-widening` cluster now fully complete. 0 new ambiguities; 9 pass / 1 pre-existing flake (unchanged). Next up: CHUNK-004 (rename-mm-package-loader).
+
+> **Session 2026-05-14**: Completed CHUNK-006 (version-bump). Bumped `Project.toml` 1.0.0 → 1.0.1 per Stated values (deliberate patch bump despite CHUNK-004 being technically breaking). No CHANGELOG present, so none updated. Test suite matches baseline; `Pkg.test` confirms `RegisterDriver v1.0.1`. All chunks in the plan are now complete. Flagged for the user: `Project.toml` also carries pre-existing unstaged `[compat]` tightening on RegisterCore/RegisterWorkerShell from before this session — bundle or split before committing. Next up: none (terminal chunk; user performs Julia registry registration).
+
+> **Session 2026-05-14**: Completed CHUNK-005 (expose-parallel-keyword). Added `parallel::Bool = length(algorithms) > 2` keyword to `driver` method 1 and replaced the implicit `usethreads = nummon > 2` switch. Default preserves existing dispatch exactly. Added a 4-test "parallel keyword" testset; all four pass when invoked directly. Dropped the originally planned 1-worker `parallel=true` case after discovering it exposes a pre-existing limitation of `driver`'s thread-pinning logic (workers run only on threads matching `workertid`, so a single worker can never see all `@threads :dynamic` iterations). Recorded in Open Questions. `Pkg.test()` shows the same baseline result (9 pass / 1 pre-existing flake; later testsets short-circuited by the flake — also pre-existing). Next up: CHUNK-006 (version-bump 1.0.0 → 1.0.1).
+
+> **Session 2026-05-14**: Completed CHUNK-004 (rename-mm-package-loader). Surfaced a collision: the originally planned target `load_mm_package` is already imported from `RegisterWorkerShell` (and is the symbol the wrapper delegates to). User chose `prepare_mm_package` as the non-colliding verb-first name. Renamed across `src/RegisterDriver.jl` (export, docstring xref, signatures, both method bodies), `README.md`, `docs/src/index.md`, and `docs/src/api.md`. Zero remaining references to the old name. Test suite identical to baseline: 9 pass / 1 pre-existing flake; Doctests/Aqua/ExplicitImports green; 0 ambiguities. Next up: CHUNK-005 (expose-parallel-keyword).
+
+## Open Questions
+- The `threadids()` test at `test/runtests.jl:88` asserts that every ID returned by `threadids()` is actually scheduled by `@threads :dynamic` workers. The Julia scheduler does not guarantee this, so the test is intermittently flaky. Separately consider relaxing the assertion (e.g., `indx ⊆ 1:nthreads()` and `length(unique(tid)) >= 1`) or removing the test. Worth handling soon: when the test fails, runtests.jl is short-circuited by Test.jl's outermost-failure path and the In-memory / parallel keyword / nicehdf5 testsets do not get to run under `Pkg.test()` (they pass when invoked directly).
+- `driver`'s threaded branch only executes worker bodies on threads matching `workertid` (`if tid in tpool`). With a single worker, only iterations that the scheduler happens to land on that one tid will actually run; the rest are silent no-ops, producing a partial or empty output file. This is the existing behaviour; CHUNK-005 surfaced it but did not change it. Consider either (a) auto-routing iterations to a free worker rather than gating by tid, or (b) documenting that `parallel=true` is only meaningful when there is one worker per active thread.

API_REVIEW_SESSION.md

@@ -0,0 +1,67 @@
+# Session Handoff — 2026-05-14
+
+## Plan
+API_REVIEW_PLAN.md — RegisterDriver, v1.0.0 → v1.0.1
+
+## What was just completed
+CHUNK-006: version-bump
+Bumped `Project.toml` from `1.0.0` → `1.0.1`. Per Stated values, the user
+deliberately chose a patch bump despite CHUNK-004 (`mm_package_loader` →
+`prepare_mm_package`) being technically breaking, on the grounds that the
+user base is small and internal. No `CHANGELOG.md` exists, so none was
+created.
+
+## Key decisions / shim choices
+- Patch bump (1.0.0 → 1.0.1), not major. Recorded in Stated values from the
+  start of the plan; CHUNK-006's intent is a release of all chunks in this
+  plan as one terminal version.
+
+## State of the codebase
+- Files modified:
+  - `Project.toml` — `version = "1.0.1"`
+  - `API_REVIEW_PLAN.md`, `API_REVIEW_SESSION.md` updated
+- Test suite: same as baseline — 9 pass / 1 pre-existing flake at
+  `runtests.jl:88`. Doctests / Aqua / ExplicitImports all pass.
+  `Pkg.test` reports `RegisterDriver v1.0.1`.
+- Ambiguity count: 0 (unchanged)
+- Staged but uncommitted: yes (see note below)
+
+## ⚠️ Pre-existing unstaged changes in Project.toml
+The working tree had unstaged `[compat]` tightening present at session
+start (carried in from before this conversation):
+
+    RegisterCore = "0.2, 1"        →  "1"
+    RegisterWorkerShell = "0.2, 1" →  "1"
+
+These are unrelated to CHUNK-006 but live in the same file. The version
+bump is staged alongside them. Decide before commit:
+
+- **Bundle**: commit Project.toml as-is. Sensible since the package is on
+  1.x and the dependencies' 0.2 lines are no longer realistic.
+- **Split**: `git restore --staged Project.toml`, then add only the
+  version line via `git add -p Project.toml`, and commit the compat
+  tightening separately (or before the bump).
+
+## Cluster status
+- annotation-widening: 2 of 2 ✓
+- All chunks complete ✓
+
+## Next chunk
+None — CHUNK-006 was terminal. After commit:
+
+1. Push to GitHub.
+2. Tag the merge commit and request registration via JuliaRegistrator
+   (`@JuliaRegistrator register` comment on the commit, or your usual
+   mechanism). The Julia registry is separate from git tags — registration
+   is its own action.
+3. TagBot will (if configured) cut the GitHub release once registration
+   merges.
+
+## Watch out for
+- The pre-existing flake at `runtests.jl:88` continues to short-circuit
+  later testsets under `Pkg.test()`. Consider relaxing/removing soon (see
+  Open Questions in the plan). This release does not address it.
+- The `driver` thread-pinning gating (`if tid in tpool`) Open Question
+  remains — out of scope for v1.0.1 but worth a follow-up plan.
+- Once you've requested registration, do not re-run `/review-implement` on
+  this plan — every chunk is `complete`.

Project.toml

@@ -1,6 +1,6 @@
 name = "RegisterDriver"
 uuid = "935ac36e-2656-11e9-1e3b-cbaa636797af"
-version = "1.0.0"
+version = "1.0.1"
 authors = ["Tim Holy <tim.holy@gmail.com>"]
 
 [deps]
@@ -26,8 +26,8 @@ HDF5 = "0.12, 0.13, 0.14, 0.15, 0.16, 0.17"
 ImageCore = "0.8.1, 0.9, 0.10"
 ImageMetadata = "0.9"
 JLD = "0.9, 0.10, 0.11, 0.12, 0.13"
-RegisterCore = "0.2, 1"
-RegisterWorkerShell = "0.2, 1"
+RegisterCore = "1"
+RegisterWorkerShell = "1"
 SharedArrays = "1"
 StaticArrays = "0.11, 0.12, 1"
 Test = "1"

README.md

@@ -87,7 +87,7 @@ Some workers require a device-specific mismatch package (e.g. a CUDA backend)
 to be loaded on the driver process before registration starts:
 
 ```julia
-mm_package_loader(algorithm)
+prepare_mm_package(algorithm)
 driver("results.jld", algorithm, img, mon)
 ```
 

docs/src/api.md

@@ -9,6 +9,6 @@ driver
 ## Utilities
 
 ```@docs
-mm_package_loader
+prepare_mm_package
 threadids
 ```

docs/src/index.md

@@ -88,7 +88,7 @@ Some workers require a device-specific mismatch package (e.g. a CUDA backend)
 to be loaded on the driver process before registration starts:
 
 ```julia
-mm_package_loader(algorithm)
+prepare_mm_package(algorithm)
 driver("results.jld", algorithm, img, mon)
 ```