docs: update AI code review findings in vendor backend story & status
This commit is contained in:
Sepehr
parent
3eb2219454
commit
c5a51d82dc
@ -0,0 +1,213 @@
|
||||
# Story 11.11: VendorBackend Trait
|
||||
|
||||
Status: done
|
||||
|
||||
<!-- Note: Validation is optional. Run validate-create-story for quality check before dev-story. -->
|
||||
|
||||
## Story
|
||||
|
||||
As a system integrator,
|
||||
I want a standardized VendorBackend trait and data types for manufacturer equipment data,
|
||||
so that I can load compressor coefficients and heat exchanger parameters from Copeland, SWEP, Danfoss, and Bitzer catalogs through a uniform API.
|
||||
|
||||
## Acceptance Criteria
|
||||
|
||||
1. **Given** a new `entropyk-vendors` crate
|
||||
**When** I add it to the workspace
|
||||
**Then** it compiles with `cargo build` and is available to `entropyk-components`
|
||||
|
||||
2. **Given** the `VendorBackend` trait
|
||||
**When** I implement it for a vendor
|
||||
**Then** it provides `list_compressor_models()`, `get_compressor_coefficients()`, `list_bphx_models()`, `get_bphx_parameters()`, and an optional `compute_ua()` with default implementation
|
||||
|
||||
3. **Given** a `CompressorCoefficients` struct
|
||||
**When** deserialized from JSON
|
||||
**Then** it contains: `model`, `manufacturer`, `refrigerant`, `capacity_coeffs: [f64; 10]`, `power_coeffs: [f64; 10]`, optional `mass_flow_coeffs`, and `ValidityRange`
|
||||
|
||||
4. **Given** a `BphxParameters` struct
|
||||
**When** deserialized from JSON
|
||||
**Then** it contains: `model`, `manufacturer`, `num_plates`, `area`, `dh`, `chevron_angle`, `ua_nominal`, and optional `UaCurve`
|
||||
|
||||
5. **Given** a `VendorError` enum
|
||||
**When** error conditions arise
|
||||
**Then** variants `ModelNotFound`, `InvalidFormat`, `FileNotFound`, `ParseError`, `IoError` are returned with descriptive messages
|
||||
|
||||
6. **Given** unit tests
|
||||
**When** `cargo test -p entropyk-vendors` is run
|
||||
**Then** serialization round-trips, trait mock implementation, UA curve interpolation, and error handling all pass
|
||||
|
||||
## Tasks / Subtasks
|
||||
|
||||
- [x] Task 1: Create `crates/vendors/` crate scaffold (AC: 1)
|
||||
- [x] Subtask 1.1: Create `crates/vendors/Cargo.toml` with `serde`, `serde_json`, `thiserror` deps
|
||||
- [x] Subtask 1.2: Add `"crates/vendors"` to workspace `members` in root `Cargo.toml`
|
||||
- [x] Subtask 1.3: Create `src/lib.rs` with module declarations and public re-exports
|
||||
- [x] Task 2: Define `VendorError` enum (AC: 5)
|
||||
- [x] Subtask 2.1: Create `src/error.rs` with thiserror-derived error variants
|
||||
- [x] Task 3: Define data types (AC: 3, 4)
|
||||
- [x] Subtask 3.1: Create `CompressorCoefficients`, `CompressorValidityRange` in `src/vendor_api.rs`
|
||||
- [x] Subtask 3.2: Create `BphxParameters`, `UaCurve`, `UaCalcParams` in `src/vendor_api.rs`
|
||||
- [x] Subtask 3.3: Derive `Serialize`, `Deserialize`, `Debug`, `Clone` on all data structs
|
||||
- [x] Task 4: Define `VendorBackend` trait (AC: 2)
|
||||
- [x] Subtask 4.1: Implement trait with 5 required methods and 1 default method in `src/vendor_api.rs`
|
||||
- [x] Subtask 4.2: Ensure trait is object-safe (`Send + Sync` bounds)
|
||||
- [x] Task 5: Create `data/` directory structure for future parsers (AC: 1)
|
||||
- [x] Subtask 5.1: Create placeholder directories: `data/copeland/compressors/`, `data/swep/bphx/`, `data/danfoss/`, `data/bitzer/`
|
||||
- [x] Subtask 5.2: Create empty `data/copeland/compressors/index.json` with `[]`
|
||||
- [x] Task 6: Create stub module files for future parser stories (AC: 1)
|
||||
- [x] Subtask 6.1: Create `src/compressors/mod.rs` with commented-out vendor imports
|
||||
- [x] Subtask 6.2: Create `src/heat_exchangers/mod.rs` with commented-out vendor imports
|
||||
- [x] Task 7: Write unit tests (AC: 6)
|
||||
- [x] Subtask 7.1: Test `CompressorCoefficients` JSON round-trip serialization
|
||||
- [x] Subtask 7.2: Test `BphxParameters` JSON round-trip serialization (with and without `UaCurve`)
|
||||
- [x] Subtask 7.3: Test `UaCurve` interpolation logic (linear interpolation between points)
|
||||
- [x] Subtask 7.4: Test `VendorError` display messages
|
||||
- [x] Subtask 7.5: Test mock `VendorBackend` implementation (list, get, error cases)
|
||||
- [x] Subtask 7.6: Test default `compute_ua` returns `ua_nominal`
|
||||
|
||||
## Dev Notes
|
||||
|
||||
### Architecture
|
||||
|
||||
**New crate:** `entropyk-vendors` — standalone crate with NO dependency on `entropyk-core`, `entropyk-fluids`, or `entropyk-components`. This is intentional: vendor data is pure data structures + I/O, not the thermodynamic engine.
|
||||
|
||||
**Integration point:** Stories 11.12-15 will implement `VendorBackend` for each vendor. The `entropyk-components` crate (or `entropyk` facade) can depend on `entropyk-vendors` to consume coefficients at system-build time.
|
||||
|
||||
**Trait design:**
|
||||
```rust
|
||||
pub trait VendorBackend: Send + Sync {
|
||||
fn vendor_name(&self) -> &str;
|
||||
fn list_compressor_models(&self) -> Result<Vec<String>, VendorError>;
|
||||
fn get_compressor_coefficients(&self, model: &str) -> Result<CompressorCoefficients, VendorError>;
|
||||
fn list_bphx_models(&self) -> Result<Vec<String>, VendorError>;
|
||||
fn get_bphx_parameters(&self, model: &str) -> Result<BphxParameters, VendorError>;
|
||||
fn compute_ua(&self, model: &str, params: &UaCalcParams) -> Result<f64, VendorError> {
|
||||
let bphx = self.get_bphx_parameters(model)?;
|
||||
Ok(bphx.ua_nominal)
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
### File Structure
|
||||
|
||||
```
|
||||
crates/vendors/
|
||||
├── Cargo.toml
|
||||
├── data/
|
||||
│ ├── copeland/compressors/index.json # empty array for now
|
||||
│ ├── swep/bphx/
|
||||
│ ├── danfoss/
|
||||
│ └── bitzer/
|
||||
└── src/
|
||||
├── lib.rs # re-exports
|
||||
├── error.rs # VendorError enum
|
||||
├── vendor_api.rs # VendorBackend trait + data types
|
||||
├── compressors/
|
||||
│ └── mod.rs # stub for future Copeland/Danfoss/Bitzer parsers
|
||||
└── heat_exchangers/
|
||||
└── mod.rs # stub for future SWEP parser
|
||||
```
|
||||
|
||||
### Key Dependencies (Cargo.toml)
|
||||
|
||||
```toml
|
||||
[package]
|
||||
name = "entropyk-vendors"
|
||||
version.workspace = true
|
||||
authors.workspace = true
|
||||
edition.workspace = true
|
||||
|
||||
[dependencies]
|
||||
serde = { version = "1.0", features = ["derive"] }
|
||||
serde_json = "1.0"
|
||||
thiserror = "1.0"
|
||||
|
||||
[dev-dependencies]
|
||||
approx = "0.5"
|
||||
```
|
||||
|
||||
### Technical Constraints
|
||||
|
||||
- **No `unwrap()`/`expect()`** — return `Result<_, VendorError>` everywhere
|
||||
- **No `println!`** — use `tracing` if logging is needed (unlikely in this story)
|
||||
- `CompressorCoefficients.capacity_coeffs` and `power_coeffs` use `[f64; 10]` (fixed-size AHRI 540 standard)
|
||||
- `mass_flow_coeffs` is `Option<[f64; 10]>` since not all vendors provide it
|
||||
- `UaCurve.points` is `Vec<(f64, f64)>` — mass_flow_ratio vs ua_ratio, linearly interpolated
|
||||
- All structs must implement `Debug + Clone + Serialize + Deserialize`
|
||||
- Trait must be object-safe for `Box<dyn VendorBackend>`
|
||||
|
||||
### AHRI 540 Coefficient Convention
|
||||
|
||||
The 10 coefficients follow the polynomial form:
|
||||
```
|
||||
C = a₀ + a₁·Ts + a₂·Td + a₃·Ts² + a₄·Ts·Td + a₅·Td²
|
||||
+ a₆·Ts³ + a₇·Td·Ts² + a₈·Ts·Td² + a₉·Td³
|
||||
```
|
||||
Where Ts = suction saturation temperature (°C), Td = discharge saturation temperature (°C). This applies to both `capacity_coeffs` (W) and `power_coeffs` (W).
|
||||
|
||||
### Previous Story Intelligence
|
||||
|
||||
**Story 11-10 (MovingBoundaryHX Cache):**
|
||||
- Used `Cell` for interior mutability inside `compute_residuals(&self)`
|
||||
- All tests pass with `cargo test -p entropyk-components`
|
||||
- Performance benchmark showed >100x speedup
|
||||
- Files modified: `crates/components/src/heat_exchanger/moving_boundary_hx.rs`, `exchanger.rs`
|
||||
|
||||
**Existing compressor implementation** at `crates/components/src/compressor.rs` (77KB) already uses AHRI 540 coefficients internally. The `CompressorCoefficients` struct in this crate should be compatible with the existing compressor configuration so vendors can feed data directly into it.
|
||||
|
||||
### Project Structure Notes
|
||||
|
||||
- New crate follows workspace conventions: `crates/vendors/`
|
||||
- Workspace root `Cargo.toml` needs `"crates/vendors"` in `members`
|
||||
- No impact on existing crates — this is purely additive
|
||||
- No Python bindings needed for this story (future story scope)
|
||||
|
||||
### References
|
||||
|
||||
- [Source: _bmad-output/planning-artifacts/epic-11-technical-specifications.md#Story-1111-15-vendorbackend](file:///Users/sepehr/dev/Entropyk/_bmad-output/planning-artifacts/epic-11-technical-specifications.md) — lines 1304-1597
|
||||
- [Source: _bmad-output/project-context.md](file:///Users/sepehr/dev/Entropyk/_bmad-output/project-context.md) — Naming conventions, error hierarchy, testing patterns
|
||||
- [Source: crates/components/src/compressor.rs](file:///Users/sepehr/dev/Entropyk/crates/components/src/compressor.rs) — Existing AHRI 540 implementation for coefficient compatibility
|
||||
- [Source: Cargo.toml](file:///Users/sepehr/dev/Entropyk/Cargo.toml) — Workspace structure
|
||||
|
||||
## Dev Agent Record
|
||||
|
||||
### Agent Model Used
|
||||
|
||||
Antigravity (Gemini)
|
||||
|
||||
### Debug Log References
|
||||
|
||||
### Review Follow-ups (AI)
|
||||
- [x] [AI-Review][High] `UaCurve::interpolate` sorting algorithm addition for parsed data.
|
||||
- [x] [AI-Review][Medium] `CompressorValidityRange` checking during parsing.
|
||||
- [x] [AI-Review][Medium] `UaCalcParams` missing derive bounds (Debug, Clone).
|
||||
- [x] [AI-Review][Low] `VendorError::IoError` missing file path tracking context.
|
||||
- [x] [AI-Review][Low] `entropyk-vendors` lib.rs missing standard `#![warn(missing_docs)]`.
|
||||
|
||||
### Completion Notes List
|
||||
|
||||
- Created `crates/vendors/` crate scaffold with `Cargo.toml`, `src/lib.rs`, module re-exports
|
||||
- Implemented `VendorError` enum in `src/error.rs` with 5 thiserror-derived variants
|
||||
- Implemented `CompressorCoefficients`, `CompressorValidityRange`, `BphxParameters`, `UaCurve`, `UaCalcParams` in `src/vendor_api.rs`
|
||||
- Implemented `UaCurve::interpolate()` with linear interpolation and endpoint clamping
|
||||
- Implemented `VendorBackend` trait with 5 required methods + 1 default `compute_ua` in `src/vendor_api.rs`
|
||||
- Created stub modules `src/compressors/mod.rs` and `src/heat_exchangers/mod.rs`
|
||||
- Created `data/` directory structure with `copeland/compressors/index.json`, `swep/bphx/`, `danfoss/`, `bitzer/`
|
||||
- Added crate to workspace `members` in root `Cargo.toml`
|
||||
- Wrote 20 unit tests covering: serialization (4), interpolation (4), error display (3), mock backend (7), default compute_ua (2), object safety (1)
|
||||
- All 20 tests pass, all core workspace crates build cleanly
|
||||
- Pre-existing `entropyk-python` build error (`missing verbose_config`) is unrelated to this story
|
||||
|
||||
### File List
|
||||
|
||||
- `crates/vendors/Cargo.toml` (new)
|
||||
- `crates/vendors/src/lib.rs` (new)
|
||||
- `crates/vendors/src/error.rs` (new)
|
||||
- `crates/vendors/src/vendor_api.rs` (new)
|
||||
- `crates/vendors/src/compressors/mod.rs` (new)
|
||||
- `crates/vendors/src/heat_exchangers/mod.rs` (new)
|
||||
- `crates/vendors/data/copeland/compressors/index.json` (new)
|
||||
- `crates/vendors/data/swep/bphx/` (new directory)
|
||||
- `crates/vendors/data/danfoss/` (new directory)
|
||||
- `crates/vendors/data/bitzer/` (new directory)
|
||||
- `Cargo.toml` (modified — added `crates/vendors` to workspace members)
|
||||
Loading…
x
Reference in New Issue
Block a user