Skip to content

Commit 1ef65d8

Browse files
authored
Merge pull request #361 from AdaWorldAPI/claude/sprint-3-spec-defect-fixes-v2
fix(specs): PR-F-1 supervisor inert-bundle skip + PR-E-1 Cargo cycle (post-#360 corrections)
2 parents b6963b5 + 3865328 commit 1ef65d8

2 files changed

Lines changed: 244 additions & 0 deletions

File tree

.claude/specs/pr-e-1-manifest-modules.md

Lines changed: 159 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -364,3 +364,162 @@ Rough breakdown:
364364
`hubspo-rs` in <30 LOC of consumer-side code
365365
- `.claude/specs/sprint-3-execution-plan.md` -- W1 master execution plan
366366
- `.claude/knowledge/tier-0-pattern-recognition.md` -- Pattern E section
367+
368+
---
369+
370+
## CORRECTION (2026-05-12, PR #360 review)
371+
372+
**Defect:** This spec said the build script in `lance-graph-contract` emits Rust glue from each manifest, including the consumer's actor type (e.g. `actor.crate: medcare-rs`, `actor.type: MedCareActor`). For the build script to emit working Rust referencing `medcare_rs::MedCareActor`, the contract crate would need `medcare-rs` as a dependency when the `module-medcare` feature is enabled. **But every consumer crate already depends on `lance-graph-contract`** (per the consumer-template W8 spec and the Single-Binary Topology I-1 invariant: consumers always pull contract). Enabling the feature therefore creates a **Cargo dependency cycle** (contract → medcare-rs → contract), which Cargo refuses to compile.
373+
374+
**Fix:** Move the concrete actor registration OUT of the contract crate. The contract crate's build script should emit ONLY:
375+
376+
1. **OGIT::* u32 namespace constants** (pure data; no consumer code referenced)
377+
2. **The `Consumer` trait declaration** (manifest-agnostic)
378+
3. **Manifest data as a static `phf::Map`** (strings + values; no Rust refs to consumer crates)
379+
380+
The actor registration moves to one of three valid mechanisms:
381+
382+
### Option A — `inventory` crate self-registration (recommended)
383+
384+
Each consumer crate impls `Consumer for ItsActor` and uses the `inventory` crate to self-register at link time:
385+
386+
```rust
387+
// crates/medcare-rs/src/actor.rs
388+
use lance_graph_contract::consumer::{Consumer, ConsumerRegistration};
389+
use inventory;
390+
391+
pub struct MedCareActor;
392+
impl Consumer for MedCareActor {
393+
const G: u32 = lance_graph_contract::OGIT::HEALTHCARE_V1.0;
394+
type Msg = MedCareMessage;
395+
fn pointer() -> ConsumerPointer { /* read from compiled-in MANIFEST_METADATA[Self::G] */ }
396+
}
397+
398+
inventory::submit! {
399+
ConsumerRegistration {
400+
g: lance_graph_contract::OGIT::HEALTHCARE_V1.0,
401+
spawn_fn: || Box::new(<MedCareActor as Consumer>::spawn()),
402+
pointer_fn: <MedCareActor as Consumer>::pointer,
403+
}
404+
}
405+
```
406+
407+
The `lance-graph-callcenter` supervisor enumerates `inventory::iter::<ConsumerRegistration>()` at startup — no compile-time generation of consumer references in the contract crate. **No dependency cycle.**
408+
409+
### Option B — umbrella-binary registration crate
410+
411+
A separate `lance-graph-binary` (or per-deployment binary crate) depends on ALL active consumer crates AND on `lance-graph-callcenter`. The build script for THIS umbrella crate (NOT the contract crate) emits:
412+
413+
```rust
414+
// crates/lance-graph-binary/src/generated/consumer_registry.rs (build-script output)
415+
pub fn register_all(supervisor: &mut CallcenterSupervisor) {
416+
supervisor.register::<medcare_rs::MedCareActor>();
417+
supervisor.register::<smb_office_rs::SmbOfficeActor>();
418+
supervisor.register::<q2::Q2CockpitActor>();
419+
// (hubspo-rs absent → not registered; G=CRM stays inert)
420+
}
421+
```
422+
423+
The contract crate stays consumer-agnostic. The umbrella crate eats the dependency-graph union. **No cycle.**
424+
425+
### Option C — callback registry at supervisor init
426+
427+
`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.
428+
429+
**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.
430+
431+
### Build-script scope correction
432+
433+
The contract crate's `build.rs` emits:
434+
435+
```rust
436+
// crates/lance-graph-contract/src/generated/ogit_namespace.rs (post-fix)
437+
pub mod OGIT {
438+
pub const DOLCE_V1: (u32, u32) = (0, 1);
439+
pub const HEALTHCARE_V1: (u32, u32) = (2, 1);
440+
pub const SMB_V1: (u32, u32) = (4, 1);
441+
pub const GOTHAM_V1: (u32, u32) = (3, 1);
442+
pub const FMA_V1: (u32, u32) = (5, 1);
443+
pub const CRM_V1: (u32, u32) = (6, 1);
444+
}
445+
446+
// crates/lance-graph-contract/src/generated/manifest_metadata.rs (post-fix)
447+
use phf::phf_map;
448+
449+
pub static MANIFEST_METADATA: phf::Map<u32, ManifestMetadata> = phf_map! {
450+
0u32 => ManifestMetadata {
451+
domain_name: "dolce",
452+
version: 1,
453+
rbac_policy_name: None,
454+
stack_profile: DomainProfile { /* ... */ },
455+
action_capabilities: &[],
456+
actor_crate: None, // inert: no consumer crate
457+
actor_type_name: None,
458+
},
459+
2u32 => ManifestMetadata {
460+
domain_name: "medcare",
461+
version: 1,
462+
rbac_policy_name: Some("medcare_policy"),
463+
stack_profile: DomainProfile { /* ... */ },
464+
action_capabilities: &[/* finalize_diagnosis: escalate, ... */],
465+
actor_crate: Some("medcare-rs"), // string only — no Rust ref
466+
actor_type_name: Some("MedCareActor"),
467+
},
468+
// ...
469+
};
470+
```
471+
472+
Every emitted symbol is **data only** — no `use medcare_rs::*` import, no actor type reference, no spawn function. The contract crate stays consumer-agnostic.
473+
474+
The **consumer-side** crate then reads its `MANIFEST_METADATA[G]` entry at compile time:
475+
476+
```rust
477+
// crates/medcare-rs/src/actor.rs (after fix)
478+
const META: &'static lance_graph_contract::ManifestMetadata =
479+
&lance_graph_contract::MANIFEST_METADATA[&lance_graph_contract::OGIT::HEALTHCARE_V1.0];
480+
481+
pub struct MedCareActor;
482+
impl Consumer for MedCareActor {
483+
const G: u32 = lance_graph_contract::OGIT::HEALTHCARE_V1.0;
484+
type Msg = MedCareMessage;
485+
fn pointer() -> ConsumerPointer {
486+
ConsumerPointer {
487+
g: Self::G,
488+
domain_name: META.domain_name.into(),
489+
stack_profile: META.stack_profile.clone(),
490+
action_capabilities: META.action_capabilities.into(),
491+
rbac_policy_ref: META.rbac_policy_name.map(|n| resolve_policy(n)),
492+
// ...
493+
}
494+
}
495+
}
496+
497+
inventory::submit! { ConsumerRegistration::new::<MedCareActor>() }
498+
```
499+
500+
### Validation
501+
502+
After this fix, the cargo dependency graph has no cycles:
503+
504+
```
505+
medcare-rs ────→ lance-graph-contract [unchanged]
506+
medcare-rs ────→ lance-graph-callcenter [unchanged]
507+
lance-graph-contract ──X (no edge to consumer crates)
508+
lance-graph-callcenter::supervisor uses inventory::iter at startup
509+
────→ lance-graph-contract (for ConsumerRegistration type)
510+
```
511+
512+
W8 consumer template stays correct — the ~50 LOC consumer scaffolding now includes:
513+
- `impl Consumer for ItsActor` block (~15 LOC)
514+
- `inventory::submit!` macro line (~5 LOC)
515+
- The actor's `Msg` enum + `Actor::handle` impl (~30 LOC)
516+
517+
**Updated PR-E-1 acceptance criteria:**
518+
519+
- [x] Build script in `lance-graph-contract` emits OGIT::* + MANIFEST_METADATA (data only)
520+
- [x] Build script does NOT reference consumer crates (no `use medcare_rs::*`)
521+
- [x] Add `inventory = "0.3"` and `phf = { version = "0.11", features = ["macros"] }` as new external deps in lance-graph-contract
522+
- [x] Document inventory pattern in W8 consumer template (see W8 spec — corrections also needed there)
523+
- [x] Verify cargo dependency graph has no cycles (`cargo tree --duplicates` clean; `cargo check --features module-medcare,module-q2-cockpit,module-smb-office` clean)
524+
525+
**Provenance:** flagged by user during PR #360 review.

.claude/specs/pr-f-1-ractor-supervisor.md

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -356,3 +356,88 @@ shader-actor message arm porting.
356356
- `.claude/specs/pr-e-1-manifest-modules.md` (W5 sister; required upstream).
357357
- `.claude/specs/sprint-3-execution-plan.md` (W1 master).
358358
- `.claude/board/sprint-log-3/agents/agent-W6.md` (this agent's log).
359+
360+
---
361+
362+
## CORRECTION (2026-05-12, PR #360 review)
363+
364+
**Defect:** The original `pre_start` loop sketched in this spec iterates over `registry.active_g_list()` and unwraps `bundle.consumer_pointer` — but inert bundles (DOLCE G=0, FMA G=5) have `consumer_pointer = None` by design. Per the W11 smoke test spec, DOLCE must remain registered as inert context (no actor) while Healthcare spawns its actor. The original loop would either panic on `unwrap()` or return `ActorProcessingErr` and abort `pre_start` before any consumer actor spawns.
365+
366+
**Fix:** Skip inert bundles in the supervisor's spawn loop. Two equivalent options:
367+
368+
### Option A — explicit filter inside `pre_start` (recommended)
369+
370+
```rust
371+
async fn pre_start(
372+
&self,
373+
myself: ActorRef<Self::Msg>,
374+
registry: Self::Arguments,
375+
) -> Result<Self::State, ActorProcessingErr> {
376+
let mut children = HashMap::new();
377+
for g in registry.all_registered_g() {
378+
let bundle = registry.resolve(g).expect("registered g must resolve");
379+
380+
// SKIP inert bundles — DOLCE / FMA / unconsumed ontologies are
381+
// queryable via SPARQL/Cypher but have no executable behavior.
382+
let pointer = match bundle.consumer_pointer.as_ref() {
383+
Some(p) => p,
384+
None => {
385+
tracing::debug!("g={} is inert (no consumer_pointer); skipping spawn", g);
386+
continue;
387+
}
388+
};
389+
390+
let (actor_ref, _handle) = Actor::spawn_linked(
391+
Some(format!("consumer_g_{}", g)),
392+
pointer.actor_type.spawn(),
393+
(),
394+
myself.get_cell(),
395+
).await?;
396+
children.insert(g, actor_ref);
397+
}
398+
Ok(children)
399+
}
400+
```
401+
402+
### Option B — narrow the iterator's contract
403+
404+
Rename `active_g_list()` to `active_consumer_g_list()` and have it return ONLY G slots whose bundle has `consumer_pointer.is_some()`. The supervisor loop becomes:
405+
406+
```rust
407+
for g in registry.active_consumer_g_list() {
408+
let bundle = registry.resolve(g).unwrap();
409+
let pointer = bundle.consumer_pointer.as_ref().unwrap(); // safe by iterator contract
410+
// ... spawn
411+
}
412+
```
413+
414+
Plus a sibling iterator `inert_g_list()` for SPARQL/Cypher consumers who need read access to all G (active + inert).
415+
416+
**Recommendation:** Option A — explicit filter — surfaces the inert-vs-active distinction at the spawn site (debugging clarity > iterator API minimalism).
417+
418+
**New test for the fix** (extends PR-F-1 test plan):
419+
420+
```rust
421+
#[tokio::test]
422+
async fn supervisor_skips_inert_bundles_and_spawns_consumers() {
423+
// Registry seeded with: DOLCE (inert), Healthcare (active), FMA (inert)
424+
let registry = test_registry_with_inert_and_active();
425+
let (sup_ref, _handle) = Actor::spawn(
426+
Some("test_sup".into()),
427+
CallcenterSupervisor { registry: registry.clone(), children: HashMap::new() },
428+
registry.clone(),
429+
).await.unwrap();
430+
431+
// Supervisor MUST be running (not aborted)
432+
assert_eq!(sup_ref.get_status(), ActorStatus::Running);
433+
434+
// Healthcare actor MUST exist; DOLCE / FMA actors MUST NOT exist
435+
assert!(supervisor_has_g(&sup_ref, OGIT::HEALTHCARE_V1.0).await);
436+
assert!(!supervisor_has_g(&sup_ref, OGIT::DOLCE_V1.0).await);
437+
assert!(!supervisor_has_g(&sup_ref, OGIT::FMA_V1.0).await);
438+
}
439+
```
440+
441+
This test also covers W11's smoke-test expectation that DOLCE is queryable but not spawned.
442+
443+
**Provenance:** flagged by user during PR #360 review.

0 commit comments

Comments
 (0)