fix(specs): PR-F-1 supervisor inert-bundle skip + PR-E-1 Cargo cycle (post-#360 corrections)#361
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 38653281e1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| `lance-graph-callcenter::supervisor::CallcenterSupervisor::with_consumers(...)` takes an explicit list of consumer types passed by the binary's `main()`. Each consumer registers itself by spec-value (no compile-time enumeration). Most explicit; no macro magic; least automation. | ||
|
|
||
| **Recommendation:** Option A (inventory crate) — best ergonomics, zero per-consumer wiring beyond the `inventory::submit!` macro, no umbrella-binary requirement. Used by `tracing` subscriber registry and many other Rust ecosystems for exactly this pattern. |
There was a problem hiding this comment.
Link inventory submitters into the final binary
With Option A, inventory::submit! records are only available from crates that are actually linked into the executable; making medcare-rs a workspace member or having it depend on lance-graph-callcenter does not cause lance-graph-callcenter/the binary to link medcare-rs. Since this correction removes the contract→consumer edge and explicitly says there is no umbrella-binary requirement, deployments that do not otherwise depend on each consumer crate will see an empty inventory::iter::<ConsumerRegistration>() and spawn no active consumers. The spec needs an explicit binary dependency/static import/features mechanism, or Option B/C should be the recommended path.
Useful? React with 👍 / 👎.
| - [x] Build script in `lance-graph-contract` emits OGIT::* + MANIFEST_METADATA (data only) | ||
| - [x] Build script does NOT reference consumer crates (no `use medcare_rs::*`) | ||
| - [x] Add `inventory = "0.3"` and `phf = { version = "0.11", features = ["macros"] }` as new external deps in lance-graph-contract | ||
| - [x] Document inventory pattern in W8 consumer template (see W8 spec — corrections also needed there) |
There was a problem hiding this comment.
Update W8 before marking inventory documented
This marks the W8 consumer-template correction as complete, but .claude/specs/consumer-crate-template.md still omits an inventory dependency in its Cargo.toml example and its actor example only implements Consumer/ConsumerPointer::from_manifest without any inventory::submit! registration. A consumer author following W8 will therefore build a crate that never registers with the new Option A supervisor path, so the acceptance checklist should not be checked until that template is corrected.
Useful? React with 👍 / 👎.
Summary
Two architectural defects in sprint-3's PR-F-1 and PR-E-1 specs (PR #360 merged) flagged during review but corrections didn't land in the merge window. This PR re-applies the corrections.
Defect 1 — PR-F-1 supervisor inert-bundle handling
The original
pre_startloop iteratesregistry.active_g_list()and unwrapsbundle.consumer_pointer. Per the W11 smoke test spec, DOLCE (G=0) and FMA (G=5) are inert bundles withconsumer_pointer = Noneby design. The original loop either panics onunwrap()or abortspre_startwithActorProcessingErrbefore any consumer actor spawns.Fix (append-only correction section at end of
pr-f-1-ractor-supervisor.md):match bundle.consumer_pointer.as_ref() { None => continue, Some(p) => spawn }filtersupervisor_skips_inert_bundles_and_spawns_consumersfor the W11 smoke-test expectationDefect 2 — PR-E-1 Cargo dependency cycle
The build script in
lance-graph-contractwas specified to emit Rust referencingmedcare_rs::MedCareActor(and similar for each compiled-in consumer). But every consumer crate depends onlance-graph-contractper Single-Binary Topology I-1 invariant. The build-script reference creates a Cargo dependency cycle (contract → medcare-rs → contract) that won't compile under--features module-medcare.Fix (append-only correction section at end of
pr-e-1-manifest-modules.md):phf::Mapof strings/values, no Rust refs to consumer crates)inventorycrate self-registration in each consumer cratewith_consumers(...)What this PR does NOT change
Why a separate PR
PR #360 merged at 21:55 just before my correction pushes landed on the sprint-3 branch (same race as PR #358/#359 sequence in sprint-2). The corrections are append-only, so a follow-up PR is the cleanest path.
Test plan
Notes
Both defects would have manifested at engineer-pickup time in sprint-4. Catching them at spec stage saves a re-spin loop.
🤖 Generated by Claude Code