From da4fdda1678ac16e6f06bfe2078525fd5963eb82 Mon Sep 17 00:00:00 2001 From: Stefanie Jane Date: Mon, 17 Aug 2026 10:16:12 -0700 Subject: [PATCH 1/7] refactor(api): share the effects response types Nine effect responses move into hypercolor-types::api::effects so the daemon and both clients read one definition instead of a struct on the daemon side and a hand-rolled decode on the client side. RescanResponse was already a struct; the other eight were anonymous serde_json::json! literals covering stop, the three effect-to-layout link routes, both control PATCH routes, the control binding PUT, and the controls reset. Shape-preserving, deliberately. serde_json is built with preserve_order in the daemon's graph, so a json! literal emits keys in literal order and a struct emits them in declaration order: every replacement struct declares its fields in the literal's exact order. A literal also emits an explicit null for a None, so no field carries skip_serializing_if, which would drop the key instead. The serde(default) markers are deserialize-only and preserve client tolerance. The web UI's private ControlsVersionResponse decoded the effect-id controls PATCH through a one-field projection; it now reads the shared UpdateEffectControlsResponse, which carries the applied and rejected control maps the projection was discarding. Co-Authored-By: Nova (Claude Opus 5) --- crates/hypercolor-daemon/src/api/effects.rs | 120 ++++++++++---------- crates/hypercolor-types/src/api/effects.rs | 106 ++++++++++++++++- crates/hypercolor-ui/src/api/effects.rs | 11 +- 3 files changed, 164 insertions(+), 73 deletions(-) diff --git a/crates/hypercolor-daemon/src/api/effects.rs b/crates/hypercolor-daemon/src/api/effects.rs index ed13f605d..b6d6bf964 100644 --- a/crates/hypercolor-daemon/src/api/effects.rs +++ b/crates/hypercolor-daemon/src/api/effects.rs @@ -10,7 +10,6 @@ use axum::http::{HeaderMap, HeaderValue, header}; use axum::response::{IntoResponse, Response}; use base64::Engine as _; use base64::engine::general_purpose::STANDARD as BASE64_STANDARD; -use serde::Serialize; use tokio::fs; use tracing::{info, warn}; @@ -64,11 +63,13 @@ pub(crate) async fn invalidate_active_render_groups_after_effect_registry_update // web UI and the TUI. pub use hypercolor_types::api::effects::{ ActiveEffectResponse, ApplyEffectPresetRequest, ApplyEffectRequest, ApplyEffectResponse, - ApplyTransitionResponse, EffectCapabilitySet, EffectDetailResponse, EffectLayoutApplyResult, - EffectListResponse, EffectPresetListResponse, EffectPresetOrigin, EffectPresetSummary, - EffectRefSummary, EffectSummary, InstalledEffectResponse, LayoutLinkSummary, - PauseEffectResponse, ResetControlsRequest, ResumeEffectResponse, TransitionRequest, - UpdateActiveControlsRequest, + ApplyTransitionResponse, DeleteEffectLayoutResponse, EffectCapabilitySet, EffectDetailResponse, + EffectLayoutApplyResult, EffectLayoutResponse, EffectListResponse, EffectPresetListResponse, + EffectPresetOrigin, EffectPresetSummary, EffectRefSummary, EffectSummary, + InstalledEffectResponse, LayoutLinkSummary, PauseEffectResponse, RescanResponse, + ResetControlsRequest, ResetControlsResponse, ResumeEffectResponse, SetControlBindingResponse, + SetEffectLayoutResponse, StopEffectResponse, TransitionRequest, UpdateActiveControlsRequest, + UpdateActiveControlsResponse, UpdateEffectControlsResponse, }; struct ResolvedEffectPreset { @@ -499,15 +500,15 @@ pub async fn get_effect_layout( }; let summary = layout.as_ref().map(layout_link_summary); - ApiResponse::ok(serde_json::json!({ - "effect": { - "id": effect_id, - "name": effect.name, + ApiResponse::ok(EffectLayoutResponse { + effect: EffectRefSummary { + id: effect_id, + name: effect.name, }, - "layout_id": layout_id, - "resolved": summary.is_some(), - "layout": summary, - })) + layout_id, + resolved: summary.is_some(), + layout: summary, + }) } /// `PUT /api/v1/effects/:id/layout` — Associate an effect with a layout. @@ -572,14 +573,14 @@ pub async fn set_effect_layout( return DomainError::Internal(anyhow::anyhow!(error)).into_response(); } - ApiResponse::ok(serde_json::json!({ - "effect": { - "id": effect_id, - "name": effect.name, + ApiResponse::ok(SetEffectLayoutResponse { + effect: EffectRefSummary { + id: effect_id, + name: effect.name, }, - "layout": layout_link_summary(&layout), - "linked": true, - })) + layout: layout_link_summary(&layout), + linked: true, + }) } /// `DELETE /api/v1/effects/:id/layout` — Remove an effect -> layout association. @@ -623,14 +624,14 @@ pub async fn delete_effect_layout( return DomainError::Internal(anyhow::anyhow!(error)).into_response(); } - ApiResponse::ok(serde_json::json!({ - "effect": { - "id": effect_id, - "name": effect.name, + ApiResponse::ok(DeleteEffectLayoutResponse { + effect: EffectRefSummary { + id: effect_id, + name: effect.name, }, - "layout_id": removed_layout_id, - "deleted": removed_layout_id.is_some(), - })) + deleted: removed_layout_id.is_some(), + layout_id: removed_layout_id, + }) } /// `POST /api/v1/effects/:id/apply` — Start rendering an effect. @@ -930,10 +931,10 @@ pub async fn stop_effect(State(state): State>) -> Response { Err(error) => return error.into_response(), }; - ApiResponse::ok(serde_json::json!({ - "stopped": true, - "released_network_devices": stopped.released_network_devices, - })) + ApiResponse::ok(StopEffectResponse { + stopped: true, + released_network_devices: stopped.released_network_devices, + }) } /// `PATCH /api/v1/effects/active/controls` — Update controls on active effect @@ -1001,11 +1002,11 @@ pub async fn update_active_controls( ); } - ApiResponse::ok(serde_json::json!({ - "effect": effect_name, - "applied": applied, - "rejected": rejected, - })) + ApiResponse::ok(UpdateActiveControlsResponse { + effect: effect_name, + applied, + rejected, + }) } /// `PATCH /api/v1/effects/{effect_id}/controls` — Update controls on a @@ -1109,12 +1110,12 @@ pub async fn update_effect_controls( ); } - let body = ApiResponse::ok(serde_json::json!({ - "effect": effect_name, - "applied": applied, - "rejected": rejected, - "controls_version": new_version, - })) + let body = ApiResponse::ok(UpdateEffectControlsResponse { + effect: effect_name, + applied, + rejected, + controls_version: new_version, + }) .into_response(); attach_controls_version_headers(body, new_version) } @@ -1218,14 +1219,14 @@ pub async fn set_active_control_binding( Err(error) => return error.into_response(), } - ApiResponse::ok(serde_json::json!({ - "effect": { - "id": effect_id, - "name": effect_name, + ApiResponse::ok(SetControlBindingResponse { + effect: EffectRefSummary { + id: effect_id, + name: effect_name, }, - "control": control_id, - "binding": normalized, - })) + control: control_id, + binding: normalized, + }) } /// `POST /api/v1/effects/active/reset` — Reset all controls on the active @@ -1270,13 +1271,13 @@ pub async fn reset_controls( info!(effect = %effect_name, "Controls reset to defaults"); - ApiResponse::ok(serde_json::json!({ - "effect": { - "id": effect_id.to_string(), - "name": effect_name, + ApiResponse::ok(ResetControlsResponse { + effect: EffectRefSummary { + id: effect_id.to_string(), + name: effect_name, }, - "reset": true, - })) + reset: true, + }) } /// `POST /api/v1/effects/rescan` — Manually trigger an effect registry rescan. @@ -1427,13 +1428,6 @@ pub async fn install_effect( }) } -#[derive(Debug, Serialize)] -pub struct RescanResponse { - pub added: usize, - pub removed: usize, - pub updated: usize, -} - pub(crate) fn resolve_effect_metadata( registry: &EffectRegistry, id_or_name: &str, diff --git a/crates/hypercolor-types/src/api/effects.rs b/crates/hypercolor-types/src/api/effects.rs index eb379dc38..346dcdaa4 100644 --- a/crates/hypercolor-types/src/api/effects.rs +++ b/crates/hypercolor-types/src/api/effects.rs @@ -6,7 +6,7 @@ use serde::{Deserialize, Serialize}; use utoipa::ToSchema; use crate::api::common::Pagination; -use crate::effect::{ControlDefinition, ControlValue, PresetTemplate}; +use crate::effect::{ControlBinding, ControlDefinition, ControlValue, PresetTemplate}; /// Origin of a preset in an effect's unified preset stack. #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize, ToSchema)] @@ -164,6 +164,17 @@ pub struct InstalledEffectResponse { pub presets: usize, } +/// Response for `POST /api/v1/effects/rescan`. +/// +/// Counts describe what the rescan changed in the registry, so an +/// all-zero response means the effect directories were already current. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct RescanResponse { + pub added: usize, + pub removed: usize, + pub updated: usize, +} + /// Request body for `POST /api/v1/effects/{id}/apply`. #[derive(Debug, Clone, Default, PartialEq, Serialize, Deserialize, ToSchema)] pub struct ApplyEffectRequest { @@ -204,6 +215,49 @@ pub struct UpdateActiveControlsRequest { pub controls: Option, } +/// Response for `PATCH /api/v1/effects/active/controls`. +/// +/// `effect` is the active effect's name rather than a reference object — +/// this route predates the `{ id, name }` convention its siblings use. +/// `rejected` names the controls the daemon refused, with the reason. +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] +pub struct UpdateActiveControlsResponse { + #[serde(default)] + pub effect: String, + #[serde(default)] + pub applied: HashMap, + #[serde(default)] + pub rejected: Vec, +} + +/// Response for `PATCH /api/v1/effects/{effect_id}/controls`. +/// +/// The same body as the `active` sibling plus `controls_version`, the +/// new server-side version token. It is also returned in the `ETag` +/// header; clients echo it back via `If-Match` to get optimistic +/// concurrency on the next PATCH. +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] +pub struct UpdateEffectControlsResponse { + #[serde(default)] + pub effect: String, + #[serde(default)] + pub applied: HashMap, + #[serde(default)] + pub rejected: Vec, + pub controls_version: u64, +} + +/// Response for `PUT /api/v1/effects/active/controls/{name}/binding`. +/// +/// `binding` is the stored binding after clamping, which can differ from +/// the one the caller sent. +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] +pub struct SetControlBindingResponse { + pub effect: EffectRefSummary, + pub control: String, + pub binding: ControlBinding, +} + /// Request body for `PUT /api/v1/effects/{id}/layout`. #[derive(Debug, Clone, Default, PartialEq, Eq, Serialize, Deserialize)] pub struct SetEffectLayoutRequest { @@ -211,6 +265,38 @@ pub struct SetEffectLayoutRequest { pub layout_id: String, } +/// Response for `GET /api/v1/effects/{id}/layout`. +/// +/// `resolved` reports whether the linked layout still exists; a stale +/// link answers `resolved: false` with a `null` `layout` rather than a +/// 404, because the association itself is real. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct EffectLayoutResponse { + pub effect: EffectRefSummary, + pub layout_id: String, + pub resolved: bool, + pub layout: Option, +} + +/// Response for `PUT /api/v1/effects/{id}/layout`. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct SetEffectLayoutResponse { + pub effect: EffectRefSummary, + pub layout: LayoutLinkSummary, + pub linked: bool, +} + +/// Response for `DELETE /api/v1/effects/{id}/layout`. +/// +/// `layout_id` is the association that was removed, and `null` with +/// `deleted: false` when the effect had no layout linked. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct DeleteEffectLayoutResponse { + pub effect: EffectRefSummary, + pub layout_id: Option, + pub deleted: bool, +} + /// Optional body for `POST /api/v1/effects/active/reset` — scopes the /// reset to one zone (`zone_id`); omitted resets the primary. #[derive(Debug, Clone, Default, PartialEq, Eq, Serialize, Deserialize, ToSchema)] @@ -219,6 +305,13 @@ pub struct ResetControlsRequest { pub zone_id: Option, } +/// Response for `POST /api/v1/effects/active/reset`. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct ResetControlsResponse { + pub effect: EffectRefSummary, + pub reset: bool, +} + /// `{ id, name }` reference to an effect. #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, ToSchema)] pub struct EffectRefSummary { @@ -244,6 +337,17 @@ pub struct ResumeEffectResponse { pub effect: Option, } +/// Response for `POST /api/v1/effects/stop`. +/// +/// `released_network_devices` counts the streaming network devices that +/// were handed back when the effect stopped. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct StopEffectResponse { + pub stopped: bool, + #[serde(default)] + pub released_network_devices: usize, +} + /// Layout link summary in apply responses. #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, ToSchema)] pub struct LayoutLinkSummary { diff --git a/crates/hypercolor-ui/src/api/effects.rs b/crates/hypercolor-ui/src/api/effects.rs index ceb1d283c..80ce762e2 100644 --- a/crates/hypercolor-ui/src/api/effects.rs +++ b/crates/hypercolor-ui/src/api/effects.rs @@ -20,6 +20,7 @@ pub use hypercolor_types::api::effects::{ ApplyEffectPresetRequest, ApplyEffectRequest as ApplyEffectBody, EffectCapabilitySet, EffectDetailResponse, EffectListResponse, EffectPresetListResponse, EffectPresetOrigin, EffectPresetSummary, EffectSummary, InstalledEffectResponse, UpdateActiveControlsRequest, + UpdateEffectControlsResponse, }; pub use hypercolor_types::api::output::{OutputPowerMode, SetOutputPowerRequest}; @@ -203,14 +204,6 @@ pub enum UpdateControlsOutcome { Stale { current: u64 }, } -/// Successful control-PATCH payload — the envelope data carries the new -/// `controls_version` (also present in the `ETag` header; the body is -/// simpler to extract with `gloo_net`). -#[derive(Debug, Deserialize)] -struct ControlsVersionResponse { - controls_version: u64, -} - /// Scoped control PATCH against a specific effect id with optional /// optimistic-concurrency precondition. /// @@ -227,7 +220,7 @@ pub async fn update_effect_controls( let body = UpdateActiveControlsRequest { controls: Some(controls.clone()), }; - let outcome = client::send_json_versioned::<_, ControlsVersionResponse>( + let outcome = client::send_json_versioned::<_, UpdateEffectControlsResponse>( Method::PATCH, &url, Some(&body), From e512be6d049806d0e80f8f80661b0522feed664f Mon Sep 17 00:00:00 2001 From: Stefanie Jane Date: Mon, 17 Aug 2026 10:16:26 -0700 Subject: [PATCH 2/7] refactor(api): share the library response types Fourteen favorites, presets, and playlists responses move into hypercolor-types::api::library. Five were daemon-local structs and nine were anonymous serde_json::json! literals: both delete acks, the favorite add ack, the preset apply ack, and the playlist activate, active, and stop acks. Field order in every replacement struct matches its literal's key order, because preserve_order makes that order part of the wire, and no field carries skip_serializing_if. The mirrors the clients kept were narrower than what the daemon sends, so deleting them widens what those clients can see. Both the web UI and the TUI dropped the pagination envelope from their favorites and preset list decodes; the TUI's favorite rows kept only effect_id, discarding the resolved effect name and the timestamp; the UI's PresetSummary retyped the preset record's controls map as raw JSON values rather than the typed ControlValue the daemon serializes. The preset routes return the stored EffectPreset verbatim, so the UI now decodes that. EffectPreset gains serde(default) on its two timestamps. The deleted UI mirror tolerated their absence, and promoting a shared type must not narrow the set of JSON a client accepts. Defaults only affect deserialization, so the daemon still serializes both unconditionally. Co-Authored-By: Nova (Claude Opus 5) --- .../src/api/library/favorites.rs | 39 ++--- .../src/api/library/playlists.rs | 56 +++---- .../src/api/library/presets.rs | 70 ++++----- crates/hypercolor-tui/src/client/rest.rs | 12 +- crates/hypercolor-types/src/api/library.rs | 143 ++++++++++++++++++ crates/hypercolor-types/src/library.rs | 2 + crates/hypercolor-ui/src/api/library.rs | 60 ++------ .../src/components/preset_panel.rs | 5 +- 8 files changed, 229 insertions(+), 158 deletions(-) diff --git a/crates/hypercolor-daemon/src/api/library/favorites.rs b/crates/hypercolor-daemon/src/api/library/favorites.rs index 840cf1301..5f1ce10a5 100644 --- a/crates/hypercolor-daemon/src/api/library/favorites.rs +++ b/crates/hypercolor-daemon/src/api/library/favorites.rs @@ -7,7 +7,6 @@ use axum::Json; use axum::extract::{Path, State}; use axum::response::{IntoResponse, Response}; use hypercolor_types::event::{HypercolorEvent, LibraryChangeKind, LibraryCollection}; -use serde::Serialize; use crate::api::AppState; use crate::api::effects::resolve_effect_metadata; @@ -16,22 +15,12 @@ use crate::domain::{DomainError, ResourceKind}; use super::unix_epoch_ms; -pub use hypercolor_types::api::library::AddFavoriteRequest; - -// ── Request / Response Types ──────────────────────────────────────────── - -#[derive(Debug, Serialize)] -pub struct FavoriteSummary { - pub effect_id: String, - pub effect_name: String, - pub added_at_ms: u64, -} - -#[derive(Debug, Serialize)] -pub struct FavoriteListResponse { - pub items: Vec, - pub pagination: crate::api::devices::Pagination, -} +// Wire contracts live in hypercolor-types::api::library — shared with +// the web UI and the TUI. +pub use hypercolor_types::api::library::{ + AddFavoriteRequest, AddFavoriteResponse, DeleteFavoriteResponse, FavoriteListResponse, + FavoriteSummary, +}; // ── Handlers ──────────────────────────────────────────────────────────── @@ -105,14 +94,14 @@ pub async fn add_favorite( kind: LibraryChangeKind::Upserted, }); - ApiResponse::ok(serde_json::json!({ - "favorite": FavoriteSummary { + ApiResponse::ok(AddFavoriteResponse { + favorite: FavoriteSummary { effect_id: favorite.effect_id.to_string(), effect_name: effect.name, added_at_ms: favorite.added_at_ms, }, - "created": !existing, - })) + created: !existing, + }) } /// `DELETE /api/v1/library/favorites/:effect` — remove a favorite by effect id/name. @@ -143,8 +132,8 @@ pub async fn remove_favorite( kind: LibraryChangeKind::Removed, }); - ApiResponse::ok(serde_json::json!({ - "effect_id": effect.id.to_string(), - "deleted": true, - })) + ApiResponse::ok(DeleteFavoriteResponse { + effect_id: effect.id.to_string(), + deleted: true, + }) } diff --git a/crates/hypercolor-daemon/src/api/library/playlists.rs b/crates/hypercolor-daemon/src/api/library/playlists.rs index d54561cf4..61cf7e0c3 100644 --- a/crates/hypercolor-daemon/src/api/library/playlists.rs +++ b/crates/hypercolor-daemon/src/api/library/playlists.rs @@ -7,7 +7,6 @@ use std::time::Duration; use axum::Json; use axum::extract::{Path, State}; use axum::response::{IntoResponse, Response}; -use serde::Serialize; use tokio::sync::watch; use tracing::warn; @@ -27,29 +26,16 @@ use super::{ store_error_to_response, unix_epoch_ms, }; +// Wire contracts live in hypercolor-types::api::library — shared with +// the web UI and the TUI. pub use hypercolor_types::api::library::{ - PlaylistItemRequest, PlaylistTargetRequest, SavePlaylistRequest, + ActivatePlaylistResponse, ActivePlaylistResponse, ActivePlaylistStateResponse, + DeletePlaylistResponse, PlaylistItemRequest, PlaylistListResponse, PlaylistTargetRequest, + SavePlaylistRequest, StopPlaylistResponse, }; const DEFAULT_PLAYLIST_ITEM_DURATION_MS: u64 = 30_000; -// ── Request / Response Types ──────────────────────────────────────────── - -#[derive(Debug, Serialize)] -pub struct PlaylistListResponse { - pub items: Vec, - pub pagination: crate::api::devices::Pagination, -} - -#[derive(Debug, Clone, Serialize)] -pub struct ActivePlaylistResponse { - pub id: String, - pub name: String, - pub loop_enabled: bool, - pub item_count: usize, - pub started_at_ms: u64, -} - // ── Handlers ──────────────────────────────────────────────────────────── /// `GET /api/v1/library/playlists` — list all playlists. @@ -216,10 +202,10 @@ pub async fn delete_playlist( }; stop_runtime(active); - ApiResponse::ok(serde_json::json!({ - "id": playlist_id.to_string(), - "deleted": true, - })) + ApiResponse::ok(DeletePlaylistResponse { + id: playlist_id.to_string(), + deleted: true, + }) } /// `POST /api/v1/library/playlists/:id/activate` — start playlist playback. @@ -284,10 +270,10 @@ pub async fn activate_playlist( runtime.active = Some(active); } - ApiResponse::ok(serde_json::json!({ - "playlist": response_payload, - "active": true, - })) + ApiResponse::ok(ActivatePlaylistResponse { + playlist: response_payload, + active: true, + }) } /// `GET /api/v1/library/playlists/active` — inspect the active playlist runtime. @@ -297,10 +283,10 @@ pub async fn get_active_playlist(State(state): State>) -> Response return DomainError::not_found(ResourceKind::Playlist, "active").into_response(); }; - ApiResponse::ok(serde_json::json!({ - "playlist": active_playlist_payload(active), - "state": "running", - })) + ApiResponse::ok(ActivePlaylistStateResponse { + playlist: active_playlist_payload(active), + state: "running".to_owned(), + }) } /// `POST /api/v1/library/playlists/stop` — stop playlist playback if active. @@ -316,10 +302,10 @@ pub async fn stop_playlist(State(state): State>) -> Response { let payload = active_playlist_payload(&active); stop_runtime(Some(active)); - ApiResponse::ok(serde_json::json!({ - "playlist": payload, - "stopped": true, - })) + ApiResponse::ok(StopPlaylistResponse { + playlist: payload, + stopped: true, + }) } // ── Helpers ───────────────────────────────────────────────────────────── diff --git a/crates/hypercolor-daemon/src/api/library/presets.rs b/crates/hypercolor-daemon/src/api/library/presets.rs index aa309e64b..b9d85373e 100644 --- a/crates/hypercolor-daemon/src/api/library/presets.rs +++ b/crates/hypercolor-daemon/src/api/library/presets.rs @@ -6,7 +6,6 @@ use std::sync::Arc; use axum::Json; use axum::extract::{Path, State}; use axum::response::{IntoResponse, Response}; -use serde::Serialize; use hypercolor_types::effect::{ControlValue, EffectMetadata}; use hypercolor_types::event::{ @@ -17,7 +16,7 @@ use hypercolor_types::scene::ZoneId; use crate::api::AppState; use crate::api::control_values::json_to_control_value; -use crate::api::effects::resolve_effect_metadata; +use crate::api::effects::{EffectRefSummary, resolve_effect_metadata}; use crate::api::envelope::ApiResponse; use crate::domain::{DomainError, ResourceKind}; @@ -26,15 +25,12 @@ use super::{ resolve_preset_id, store_error_to_response, unix_epoch_ms, }; -pub use hypercolor_types::api::library::{ApplyPresetRequest, SavePresetRequest}; - -// ── Request / Response Types ──────────────────────────────────────────── - -#[derive(Debug, Serialize)] -pub struct PresetListResponse { - pub items: Vec, - pub pagination: crate::api::devices::Pagination, -} +// Wire contracts live in hypercolor-types::api::library — shared with +// the web UI and the TUI. +pub use hypercolor_types::api::library::{ + ApplyPresetRequest, ApplyPresetResponse, DeletePresetResponse, PresetListResponse, + PresetRefSummary, SavePresetRequest, +}; // ── Handlers ──────────────────────────────────────────────────────────── @@ -203,10 +199,10 @@ pub async fn delete_preset(State(state): State>, Path(id): Path::new(), - })) + applied_controls: applied, + rejected_controls: rejected, + warnings: Vec::new(), + }) } // ── Helpers ───────────────────────────────────────────────────────────── diff --git a/crates/hypercolor-tui/src/client/rest.rs b/crates/hypercolor-tui/src/client/rest.rs index 66d8d745a..bd8968ce7 100644 --- a/crates/hypercolor-tui/src/client/rest.rs +++ b/crates/hypercolor-tui/src/client/rest.rs @@ -14,7 +14,7 @@ use hypercolor_types::api::effects::{ }; use hypercolor_types::api::envelope::ApiErrorBody; use hypercolor_types::api::layers::PatchLayerControlsRequest; -use hypercolor_types::api::library::AddFavoriteRequest; +use hypercolor_types::api::library::{AddFavoriteRequest, FavoriteListResponse}; use hypercolor_types::api::scenes::{ ActiveSceneResponse as ApiActiveSceneResponse, SceneListResponse as ApiSceneListResponse, }; @@ -531,16 +531,6 @@ struct ControlSurfaceListResponse { surfaces: Vec, } -#[derive(Debug, Deserialize)] -struct FavoriteListResponse { - items: Vec, -} - -#[derive(Debug, Deserialize)] -struct FavoriteSummaryResponse { - effect_id: String, -} - #[derive(Debug, Deserialize)] struct SystemStatusResponse { running: bool, diff --git a/crates/hypercolor-types/src/api/library.rs b/crates/hypercolor-types/src/api/library.rs index e814d1dd9..1ebbd19e5 100644 --- a/crates/hypercolor-types/src/api/library.rs +++ b/crates/hypercolor-types/src/api/library.rs @@ -1,7 +1,14 @@ //! Library API contracts — `/api/v1/library/*`. +use std::collections::HashMap; + use serde::{Deserialize, Serialize}; +use crate::api::common::Pagination; +use crate::api::effects::EffectRefSummary; +use crate::effect::ControlValue; +use crate::library::{EffectPlaylist, EffectPreset}; + /// Request body for `POST /api/v1/library/favorites`. #[derive(Debug, Clone, Default, PartialEq, Eq, Serialize, Deserialize)] pub struct AddFavoriteRequest { @@ -9,6 +16,44 @@ pub struct AddFavoriteRequest { pub effect: String, } +/// One favorited effect. +/// +/// `effect_name` is resolved from the registry at request time and falls +/// back to the id when the effect is no longer installed. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct FavoriteSummary { + pub effect_id: String, + #[serde(default)] + pub effect_name: String, + #[serde(default)] + pub added_at_ms: u64, +} + +/// Response for `GET /api/v1/library/favorites`. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct FavoriteListResponse { + #[serde(default)] + pub items: Vec, + pub pagination: Pagination, +} + +/// Response for `POST /api/v1/library/favorites`. +/// +/// `created` is false when the effect was already favorited, which +/// re-stamps `added_at_ms` rather than erroring. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct AddFavoriteResponse { + pub favorite: FavoriteSummary, + pub created: bool, +} + +/// Response for `DELETE /api/v1/library/favorites/{effect}`. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct DeleteFavoriteResponse { + pub effect_id: String, + pub deleted: bool, +} + /// What one playlist item plays. #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] #[serde(tag = "type", rename_all = "snake_case")] @@ -63,3 +108,101 @@ pub struct ApplyPresetRequest { #[serde(default, skip_serializing_if = "Option::is_none")] pub zone_id: Option, } + +/// Response for `GET /api/v1/library/presets`. +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] +pub struct PresetListResponse { + #[serde(default)] + pub items: Vec, + pub pagination: Pagination, +} + +/// Response for `DELETE /api/v1/library/presets/{id}`. +/// +/// `id` is the resolved preset id, which differs from the path segment +/// when the caller addressed the preset by name. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct DeletePresetResponse { + pub id: String, + pub deleted: bool, +} + +/// `{ id, name }` reference to a saved preset. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct PresetRefSummary { + pub id: String, + pub name: String, +} + +/// Response for `POST /api/v1/library/presets/{id}/apply`. +/// +/// `applied_controls` is what the effect actually took, and +/// `rejected_controls` names the preset entries the effect's current +/// control definitions refused — a preset saved against an older version +/// of an effect applies partially rather than failing. +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] +pub struct ApplyPresetResponse { + pub preset: PresetRefSummary, + pub effect: EffectRefSummary, + #[serde(default)] + pub applied_controls: HashMap, + #[serde(default)] + pub rejected_controls: Vec, + #[serde(default)] + pub warnings: Vec, +} + +/// Response for `GET /api/v1/library/playlists`. +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] +pub struct PlaylistListResponse { + #[serde(default)] + pub items: Vec, + pub pagination: Pagination, +} + +/// Response for `DELETE /api/v1/library/playlists/{id}`. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct DeletePlaylistResponse { + pub id: String, + pub deleted: bool, +} + +/// The playlist the daemon is currently cycling through. +/// +/// This is the live runtime's view, not the stored playlist: the item +/// list is reduced to `item_count`, and `started_at_ms` is when playback +/// began rather than when the playlist was saved. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct ActivePlaylistResponse { + pub id: String, + pub name: String, + pub loop_enabled: bool, + pub item_count: usize, + pub started_at_ms: u64, +} + +/// Response for `POST /api/v1/library/playlists/{id}/activate`. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct ActivatePlaylistResponse { + pub playlist: ActivePlaylistResponse, + pub active: bool, +} + +/// Response for `GET /api/v1/library/playlists/active`. +/// +/// The route answers 404 when nothing is playing, so `state` is always +/// `"running"` on a success. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct ActivePlaylistStateResponse { + pub playlist: ActivePlaylistResponse, + #[serde(default)] + pub state: String, +} + +/// Response for `POST /api/v1/library/playlists/stop`. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct StopPlaylistResponse { + /// The playlist as it stood when playback stopped. + pub playlist: ActivePlaylistResponse, + pub stopped: bool, +} diff --git a/crates/hypercolor-types/src/library.rs b/crates/hypercolor-types/src/library.rs index 1182b5214..2d1f70c77 100644 --- a/crates/hypercolor-types/src/library.rs +++ b/crates/hypercolor-types/src/library.rs @@ -162,7 +162,9 @@ pub struct EffectPreset { pub controls: HashMap, #[serde(default)] pub tags: Vec, + #[serde(default)] pub created_at_ms: u64, + #[serde(default)] pub updated_at_ms: u64, } diff --git a/crates/hypercolor-ui/src/api/library.rs b/crates/hypercolor-ui/src/api/library.rs index c6f8d345c..31fb8e7ff 100644 --- a/crates/hypercolor-ui/src/api/library.rs +++ b/crates/hypercolor-ui/src/api/library.rs @@ -1,70 +1,34 @@ //! Library API — presets and favorites. -use serde::{Deserialize, Serialize}; -use std::collections::HashMap; - use super::client; -// ── Preset Types ──────────────────────────────────────────────────────────── - -/// Preset summary from `GET /api/v1/library/presets`. -#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] -pub struct PresetSummary { - pub id: String, - pub name: String, - pub description: Option, - pub effect_id: String, - #[serde(default)] - pub controls: HashMap, - #[serde(default)] - pub tags: Vec, - #[serde(default)] - pub created_at_ms: u64, - #[serde(default)] - pub updated_at_ms: u64, -} - -/// Paginated preset list response. -#[derive(Debug, Deserialize)] -pub struct PresetListResponse { - pub items: Vec, -} - -pub use hypercolor_types::api::library::{AddFavoriteRequest, SavePresetRequest}; - -// ── Favorite Types ────────────────────────────────────────────────────────── - -/// Favorite entry from `GET /api/v1/library/favorites`. -#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] -pub struct FavoriteSummary { - pub effect_id: String, - pub effect_name: String, - pub added_at_ms: u64, -} - -/// Paginated favorites list response. -#[derive(Debug, Deserialize)] -pub struct FavoriteListResponse { - pub items: Vec, -} +// Wire contracts are shared with the daemon +// (hypercolor-types::api::library) — drift is a compile error rather +// than a runtime parse failure. `EffectPreset` is the daemon's stored +// preset record, returned verbatim by the preset routes. +pub use hypercolor_types::api::library::{ + AddFavoriteRequest, FavoriteListResponse, FavoriteSummary, PresetListResponse, + SavePresetRequest, +}; +pub use hypercolor_types::library::EffectPreset; // ── Preset Functions ──────────────────────────────────────────────────────── /// Fetch all saved presets. -pub async fn fetch_presets() -> Result, String> { +pub async fn fetch_presets() -> Result, String> { let list: PresetListResponse = client::fetch_json("/api/v1/library/presets").await?; Ok(list.items) } /// Create a new preset from current control values. -pub async fn create_preset(req: &SavePresetRequest) -> Result { +pub async fn create_preset(req: &SavePresetRequest) -> Result { client::post_json("/api/v1/library/presets", req) .await .map_err(Into::into) } /// Update an existing preset (name, controls, etc.). -pub async fn update_preset(id: &str, req: &SavePresetRequest) -> Result { +pub async fn update_preset(id: &str, req: &SavePresetRequest) -> Result { client::put_json(&format!("/api/v1/library/presets/{id}"), req) .await .map_err(Into::into) diff --git a/crates/hypercolor-ui/src/components/preset_panel.rs b/crates/hypercolor-ui/src/components/preset_panel.rs index f338e6525..810594cc5 100644 --- a/crates/hypercolor-ui/src/components/preset_panel.rs +++ b/crates/hypercolor-ui/src/components/preset_panel.rs @@ -264,10 +264,11 @@ pub fn PresetToolbar( }; match api::create_preset(&req).await { Ok(created) => { - match api::apply_effect_preset(&eid, &created.id, target_zone.as_deref()).await + let created_id = created.id.to_string(); + match api::apply_effect_preset(&eid, &created_id, target_zone.as_deref()).await { Ok(()) => { - set_selected_id.set(Some(created.id)); + set_selected_id.set(Some(created_id)); toasts::toast_success("Preset created"); refresh(); } From 8c1c8f2944aa552ffc8042a49c36a456975c2b88 Mon Sep 17 00:00:00 2001 From: Stefanie Jane Date: Mon, 17 Aug 2026 10:16:35 -0700 Subject: [PATCH 3/7] refactor(api): share the attachment template catalog types The template catalog's eight response shapes move into hypercolor-types::api::attachments: the listing and its summary, the single-template detail returned by GET, POST, and PUT, the delete ack that was a json! literal, and the category and vendor facet lists. Declaration order matches the daemon's prior order in every case. This closes real drift rather than only deduplicating. The web UI's TemplateSummary mirror had no image_url field at all, so the catalog's artwork was invisible to it, and it typed origin as an Option where the daemon sends the value unconditionally, which made every built-in template indistinguishable from a user-authored one at the type level. Its TemplateListResponse dropped the pagination envelope, leaving the UI unable to page a catalog it fetches with a hardcoded limit of 200. Deleting those mirrors hands the UI all three. The daemon's bytes do not move: these fields were always on the wire, only unread. create_attachment_template also decoded the POST response, which is a full TemplateDetail, into the narrower summary. It now decodes the detail, so a freshly created template's topology and LED positions survive the round trip. Co-Authored-By: Nova (Claude Opus 5) --- .../hypercolor-daemon/src/api/attachments.rs | 78 ++---------- .../hypercolor-types/src/api/attachments.rs | 112 ++++++++++++++++++ crates/hypercolor-ui/src/api/devices.rs | 27 +---- .../src/components/attachment_editor.rs | 4 +- .../tests/attachment_editor_tests.rs | 5 +- .../tests/component_picker_tests.rs | 5 +- 6 files changed, 135 insertions(+), 96 deletions(-) diff --git a/crates/hypercolor-daemon/src/api/attachments.rs b/crates/hypercolor-daemon/src/api/attachments.rs index b9ceaed3a..1836c9e5f 100644 --- a/crates/hypercolor-daemon/src/api/attachments.rs +++ b/crates/hypercolor-daemon/src/api/attachments.rs @@ -7,7 +7,6 @@ use std::sync::Arc; use axum::Json; use axum::extract::{Path as AxumPath, Query, State}; use axum::response::{IntoResponse, Response}; -use serde::Serialize; use tokio::sync::RwLockWriteGuard; use hypercolor_core::attachment::{ComponentRegistry, TemplateFilter}; @@ -22,69 +21,12 @@ use crate::api::devices::Pagination; use crate::api::envelope::ApiResponse; use crate::domain::{DomainError, ResourceKind}; -pub use hypercolor_types::api::attachments::ListTemplatesQuery; - -#[derive(Debug, Serialize)] -pub struct TemplateListResponse { - pub items: Vec, - pub pagination: Pagination, -} - -#[derive(Debug, Clone, Serialize)] -pub struct TemplateSummary { - pub id: String, - pub name: String, - pub vendor: String, - pub category: ComponentCategory, - pub origin: ComponentOrigin, - pub led_count: u32, - pub description: String, - pub image_url: Option, - pub tags: Vec, -} - -#[derive(Debug, Serialize)] -pub struct TemplateDetail { - pub id: String, - pub name: String, - pub vendor: String, - pub category: ComponentCategory, - pub origin: ComponentOrigin, - pub led_count: u32, - pub description: String, - pub default_size: hypercolor_types::attachment::ComponentCanvasSize, - pub topology: hypercolor_types::spatial::LedTopology, - pub led_positions: Vec, - pub compatible_slots: Vec, - pub tags: Vec, - pub led_names: Option>, - pub led_mapping: Option>, - pub image_url: Option, - pub physical_size_mm: Option<(f32, f32)>, -} - -#[derive(Debug, Serialize)] -pub struct CategoryListResponse { - pub items: Vec, -} - -#[derive(Debug, Serialize)] -pub struct CategorySummary { - pub category: ComponentCategory, - pub count: usize, - pub label: String, -} - -#[derive(Debug, Serialize)] -pub struct VendorListResponse { - pub items: Vec, -} - -#[derive(Debug, Serialize)] -pub struct VendorSummary { - pub vendor: String, - pub count: usize, -} +// Wire contracts live in hypercolor-types::api::attachments — shared +// with the web UI and the TUI. +pub use hypercolor_types::api::attachments::{ + CategoryListResponse, CategorySummary, DeleteTemplateResponse, ListTemplatesQuery, + TemplateDetail, TemplateListResponse, TemplateSummary, VendorListResponse, VendorSummary, +}; /// `GET /api/v1/attachments/templates` pub async fn list_templates( @@ -220,10 +162,10 @@ pub async fn delete_template( return DomainError::Internal(anyhow::anyhow!("{error}")).into_response(); } - ApiResponse::ok(serde_json::json!({ - "id": removed.id, - "deleted": true, - })) + ApiResponse::ok(DeleteTemplateResponse { + id: removed.id, + deleted: true, + }) } /// `GET /api/v1/attachments/categories` diff --git a/crates/hypercolor-types/src/api/attachments.rs b/crates/hypercolor-types/src/api/attachments.rs index 1696d0b1a..17b1328df 100644 --- a/crates/hypercolor-types/src/api/attachments.rs +++ b/crates/hypercolor-types/src/api/attachments.rs @@ -2,6 +2,12 @@ use serde::{Deserialize, Serialize}; +use crate::api::common::Pagination; +use crate::attachment::{ + ComponentCanvasSize, ComponentCategory, ComponentCompatibility, ComponentOrigin, +}; +use crate::spatial::{LedTopology, NormalizedPosition}; + /// Query parameters for `GET /api/v1/attachments/templates`. /// /// Every field narrows the catalog; an empty query lists everything the @@ -33,3 +39,109 @@ pub struct ListTemplatesQuery { #[serde(default, skip_serializing_if = "Option::is_none")] pub led_max: Option, } + +/// Response for `GET /api/v1/attachments/templates`. +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] +pub struct TemplateListResponse { + #[serde(default)] + pub items: Vec, + pub pagination: Pagination, +} + +/// One template in the catalog listing. +/// +/// `led_count` is the template's resolved LED total, derived from its +/// topology rather than stored, so it is always present even for +/// templates whose topology is generated. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct TemplateSummary { + pub id: String, + pub name: String, + pub vendor: String, + pub category: ComponentCategory, + #[serde(default)] + pub origin: ComponentOrigin, + pub led_count: u32, + pub description: String, + #[serde(default)] + pub image_url: Option, + #[serde(default)] + pub tags: Vec, +} + +/// Response for `GET`, `POST`, and `PUT` on a single template. +/// +/// The summary's fields plus everything needed to place the attachment: +/// `led_positions` is expanded from the topology at request time, so it +/// is present here but never in the listing. +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] +pub struct TemplateDetail { + pub id: String, + pub name: String, + pub vendor: String, + pub category: ComponentCategory, + #[serde(default)] + pub origin: ComponentOrigin, + pub led_count: u32, + pub description: String, + pub default_size: ComponentCanvasSize, + pub topology: LedTopology, + #[serde(default)] + pub led_positions: Vec, + #[serde(default)] + pub compatible_slots: Vec, + #[serde(default)] + pub tags: Vec, + #[serde(default)] + pub led_names: Option>, + #[serde(default)] + pub led_mapping: Option>, + #[serde(default)] + pub image_url: Option, + /// Physical footprint in millimeters, as `[width, height]`. + #[serde(default)] + pub physical_size_mm: Option<(f32, f32)>, +} + +/// Response for `DELETE /api/v1/attachments/templates/{id}`. +/// +/// Built-in templates cannot be deleted, so a success here always means +/// a user-authored template was removed. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct DeleteTemplateResponse { + pub id: String, + pub deleted: bool, +} + +/// Response for `GET /api/v1/attachments/categories`. +/// +/// Unpaginated: the category set is bounded by the catalog's own +/// vocabulary rather than by how many templates are installed. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct CategoryListResponse { + #[serde(default)] + pub items: Vec, +} + +/// One category and how many templates carry it. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct CategorySummary { + pub category: ComponentCategory, + pub count: usize, + /// Display-ready category name, titleized for unknown categories. + pub label: String, +} + +/// Response for `GET /api/v1/attachments/vendors`. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct VendorListResponse { + #[serde(default)] + pub items: Vec, +} + +/// One vendor and how many templates it has in the catalog. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct VendorSummary { + pub vendor: String, + pub count: usize, +} diff --git a/crates/hypercolor-ui/src/api/devices.rs b/crates/hypercolor-ui/src/api/devices.rs index b4644c8de..16f97f44d 100644 --- a/crates/hypercolor-ui/src/api/devices.rs +++ b/crates/hypercolor-ui/src/api/devices.rs @@ -30,26 +30,11 @@ pub struct BrightnessSettingsResponse { // ── Attachment Types ──────────────────────────────────────────────────────── -/// Template summary from `GET /api/v1/attachments/templates`. -#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] -pub struct TemplateSummary { - pub id: String, - pub name: String, - pub vendor: String, - pub category: hypercolor_types::attachment::ComponentCategory, - #[serde(default)] - pub origin: Option, - pub led_count: u32, - pub description: String, - #[serde(default)] - pub tags: Vec, -} - -/// Paginated template list response. -#[derive(Debug, Deserialize)] -pub struct TemplateListResponse { - pub items: Vec, -} +// The attachment-template catalog contracts are shared with the daemon +// (hypercolor-types::api::attachments). +pub use hypercolor_types::api::attachments::{ + TemplateDetail, TemplateListResponse, TemplateSummary, +}; // ── Fetch Functions ───────────────────────────────────────────────────────── @@ -123,7 +108,7 @@ pub async fn identify_attachment( /// Create a user-authored attachment template (custom strip, matrix, etc.). pub async fn create_attachment_template( template: &hypercolor_types::attachment::ComponentTemplate, -) -> Result { +) -> Result { client::post_json("/api/v1/attachments/templates", template) .await .map_err(Into::into) diff --git a/crates/hypercolor-ui/src/components/attachment_editor.rs b/crates/hypercolor-ui/src/components/attachment_editor.rs index 97ab03cf8..b10c3aca0 100644 --- a/crates/hypercolor-ui/src/components/attachment_editor.rs +++ b/crates/hypercolor-ui/src/components/attachment_editor.rs @@ -176,9 +176,7 @@ pub fn expand_bindings_to_drafts( // Check if this is a user-created strip/matrix template let tmpl = templates.iter().find(|t| t.id == binding.template_id); let is_user = tmpl - .and_then(|t| t.origin.as_ref()) - .map(|o| *o == hypercolor_types::attachment::ComponentOrigin::User) - .unwrap_or(false); + .is_some_and(|t| t.origin == hypercolor_types::attachment::ComponentOrigin::User); if is_user { // Reconstruct as inline strip/matrix based on category diff --git a/crates/hypercolor-ui/tests/attachment_editor_tests.rs b/crates/hypercolor-ui/tests/attachment_editor_tests.rs index 95d7b931e..e650da84d 100644 --- a/crates/hypercolor-ui/tests/attachment_editor_tests.rs +++ b/crates/hypercolor-ui/tests/attachment_editor_tests.rs @@ -1,4 +1,4 @@ -use hypercolor_types::attachment::{ComponentCategory, ComponentSlot}; +use hypercolor_types::attachment::{ComponentCategory, ComponentOrigin, ComponentSlot}; use hypercolor_ui::api::{ComponentBindingSummary, TemplateSummary}; use hypercolor_ui::components::attachment_editor::{ DraftRow, expand_bindings_to_drafts, summarize_channel, @@ -22,9 +22,10 @@ fn template(id: &str, name: &str, category: ComponentCategory, led_count: u32) - name: name.to_owned(), vendor: "Lian Li".to_owned(), category, - origin: None, + origin: ComponentOrigin::BuiltIn, led_count, description: String::new(), + image_url: None, tags: Vec::new(), } } diff --git a/crates/hypercolor-ui/tests/component_picker_tests.rs b/crates/hypercolor-ui/tests/component_picker_tests.rs index b90c0f0cc..c9f41f038 100644 --- a/crates/hypercolor-ui/tests/component_picker_tests.rs +++ b/crates/hypercolor-ui/tests/component_picker_tests.rs @@ -1,4 +1,4 @@ -use hypercolor_types::attachment::ComponentCategory; +use hypercolor_types::attachment::{ComponentCategory, ComponentOrigin}; use hypercolor_ui::api::TemplateSummary; use hypercolor_ui::components::component_picker::{filter_components, selected_result_index}; @@ -8,9 +8,10 @@ fn template(id: &str, name: &str, vendor: &str, category: ComponentCategory) -> name: name.to_owned(), vendor: vendor.to_owned(), category, - origin: None, + origin: ComponentOrigin::BuiltIn, led_count: 16, description: String::new(), + image_url: None, tags: Vec::new(), } } From 8b35c2ce7b74c8e0af37248c87609c267615d2a8 Mon Sep 17 00:00:00 2001 From: Stefanie Jane Date: Mon, 17 Aug 2026 11:54:37 -0700 Subject: [PATCH 4/7] fix(api): keep the f32-bearing control responses unpromoted Adversarial review caught a wire change in the first pass. `json!` routes every value through `serde_json::to_value`, which stores an f32 as f64, and the widening is visible in the printed digits: 0.1f32 becomes 0.10000000149011612. A derived struct writes the f32 straight out as 0.1. So promoting a literal that carries a float reprints it, on top of the key-order and explicit-null hazards the wave already accounted for. Four responses carry f32 and stay literals: both control PATCH bodies, the control binding PUT, and the preset apply. Their payloads are `ControlValue` and `ControlBinding`, so the reprint would hit every slider, color, gradient stop, and binding bound rather than some edge case. Each site says why it was left alone, and the web UI keeps its narrow controls-version decode for the same reason. The remaining twenty-one promotions are byte-identical and unaffected. The list responses now mark `pagination` with serde(default), which needs Default on Pagination. The client mirrors this wave deletes had no pagination field at all and ignored the envelope, so without the default a body omitting it would newly fail to parse in both the web UI and the TUI. Pagination's explanation sits in a line comment because utoipa publishes the doc comment as the schema description, and editing it moves the generated OpenAPI client. Also documents why the UI keeps a local ActiveEffectResponse: it is a projection that unwraps the wire type's Option id and name for consumers that have already branched on `state`, not a mirror of the wire shape. Co-Authored-By: Nova (Claude Opus 5) --- crates/hypercolor-daemon/src/api/effects.rs | 49 +++++++++-------- .../src/api/library/presets.rs | 54 ++++++++++--------- .../hypercolor-types/src/api/attachments.rs | 1 + crates/hypercolor-types/src/api/common.rs | 7 ++- crates/hypercolor-types/src/api/effects.rs | 49 +++-------------- crates/hypercolor-types/src/api/library.rs | 35 +++--------- crates/hypercolor-ui/src/api/effects.rs | 26 +++++++-- 7 files changed, 100 insertions(+), 121 deletions(-) diff --git a/crates/hypercolor-daemon/src/api/effects.rs b/crates/hypercolor-daemon/src/api/effects.rs index b6d6bf964..58f591ac5 100644 --- a/crates/hypercolor-daemon/src/api/effects.rs +++ b/crates/hypercolor-daemon/src/api/effects.rs @@ -67,9 +67,8 @@ pub use hypercolor_types::api::effects::{ EffectLayoutApplyResult, EffectLayoutResponse, EffectListResponse, EffectPresetListResponse, EffectPresetOrigin, EffectPresetSummary, EffectRefSummary, EffectSummary, InstalledEffectResponse, LayoutLinkSummary, PauseEffectResponse, RescanResponse, - ResetControlsRequest, ResetControlsResponse, ResumeEffectResponse, SetControlBindingResponse, - SetEffectLayoutResponse, StopEffectResponse, TransitionRequest, UpdateActiveControlsRequest, - UpdateActiveControlsResponse, UpdateEffectControlsResponse, + ResetControlsRequest, ResetControlsResponse, ResumeEffectResponse, SetEffectLayoutResponse, + StopEffectResponse, TransitionRequest, UpdateActiveControlsRequest, }; struct ResolvedEffectPreset { @@ -1002,11 +1001,15 @@ pub async fn update_active_controls( ); } - ApiResponse::ok(UpdateActiveControlsResponse { - effect: effect_name, - applied, - rejected, - }) + // Held back from the wave 3.1c type promotion deliberately: `applied` + // carries f32 control values, and `json!` widens them to f64 while a + // derived struct does not, so naming this shape would reprint every + // non-representable float (0.1 -> 0.10000000149011612 today). + ApiResponse::ok(serde_json::json!({ + "effect": effect_name, + "applied": applied, + "rejected": rejected, + })) } /// `PATCH /api/v1/effects/{effect_id}/controls` — Update controls on a @@ -1110,12 +1113,13 @@ pub async fn update_effect_controls( ); } - let body = ApiResponse::ok(UpdateEffectControlsResponse { - effect: effect_name, - applied, - rejected, - controls_version: new_version, - }) + // Held back for the same f32 reprint reason as its `active` sibling. + let body = ApiResponse::ok(serde_json::json!({ + "effect": effect_name, + "applied": applied, + "rejected": rejected, + "controls_version": new_version, + })) .into_response(); attach_controls_version_headers(body, new_version) } @@ -1219,14 +1223,17 @@ pub async fn set_active_control_binding( Err(error) => return error.into_response(), } - ApiResponse::ok(SetControlBindingResponse { - effect: EffectRefSummary { - id: effect_id, - name: effect_name, + // Held back: `ControlBinding` is six f32 fields, which a named struct + // would reprint at f32 precision instead of the widened f64 form the + // literal has always emitted. + ApiResponse::ok(serde_json::json!({ + "effect": { + "id": effect_id, + "name": effect_name, }, - control: control_id, - binding: normalized, - }) + "control": control_id, + "binding": normalized, + })) } /// `POST /api/v1/effects/active/reset` — Reset all controls on the active diff --git a/crates/hypercolor-daemon/src/api/library/presets.rs b/crates/hypercolor-daemon/src/api/library/presets.rs index b9d85373e..b0d801982 100644 --- a/crates/hypercolor-daemon/src/api/library/presets.rs +++ b/crates/hypercolor-daemon/src/api/library/presets.rs @@ -16,7 +16,7 @@ use hypercolor_types::scene::ZoneId; use crate::api::AppState; use crate::api::control_values::json_to_control_value; -use crate::api::effects::{EffectRefSummary, resolve_effect_metadata}; +use crate::api::effects::resolve_effect_metadata; use crate::api::envelope::ApiResponse; use crate::domain::{DomainError, ResourceKind}; @@ -28,8 +28,7 @@ use super::{ // Wire contracts live in hypercolor-types::api::library — shared with // the web UI and the TUI. pub use hypercolor_types::api::library::{ - ApplyPresetRequest, ApplyPresetResponse, DeletePresetResponse, PresetListResponse, - PresetRefSummary, SavePresetRequest, + ApplyPresetRequest, DeletePresetResponse, PresetListResponse, SavePresetRequest, }; // ── Handlers ──────────────────────────────────────────────────────────── @@ -325,19 +324,23 @@ pub async fn apply_preset( }; crate::api::persist_runtime_session(&state).await; - ApiResponse::ok(ApplyPresetResponse { - preset: PresetRefSummary { - id: preset.id.to_string(), - name: preset.name, + // Held back from the wave 3.1c type promotion deliberately: + // `applied_controls` carries f32 control values, and `json!` widens + // them to f64 while a derived struct does not, so naming this shape + // would reprint every non-representable float. + ApiResponse::ok(serde_json::json!({ + "preset": { + "id": preset.id.to_string(), + "name": preset.name, }, - effect: EffectRefSummary { - id: metadata.id.to_string(), - name: metadata.name, + "effect": { + "id": metadata.id.to_string(), + "name": metadata.name, }, - applied_controls: activation.applied, - rejected_controls: activation.rejected, - warnings: activation.warnings, - }) + "applied_controls": activation.applied, + "rejected_controls": activation.rejected, + "warnings": activation.warnings, + })) } /// Apply a preset to a named non-Primary zone. When the zone already runs @@ -425,19 +428,20 @@ async fn apply_preset_to_zone( } crate::api::persist_runtime_session(state).await; - ApiResponse::ok(ApplyPresetResponse { - preset: PresetRefSummary { - id: preset.id.to_string(), - name: preset.name.clone(), + // Held back for the same f32 reprint reason as the primary path. + ApiResponse::ok(serde_json::json!({ + "preset": { + "id": preset.id.to_string(), + "name": preset.name, }, - effect: EffectRefSummary { - id: metadata.id.to_string(), - name: metadata.name.clone(), + "effect": { + "id": metadata.id.to_string(), + "name": metadata.name, }, - applied_controls: applied, - rejected_controls: rejected, - warnings: Vec::new(), - }) + "applied_controls": applied, + "rejected_controls": rejected, + "warnings": Vec::::new(), + })) } // ── Helpers ───────────────────────────────────────────────────────────── diff --git a/crates/hypercolor-types/src/api/attachments.rs b/crates/hypercolor-types/src/api/attachments.rs index 17b1328df..7f858f78c 100644 --- a/crates/hypercolor-types/src/api/attachments.rs +++ b/crates/hypercolor-types/src/api/attachments.rs @@ -45,6 +45,7 @@ pub struct ListTemplatesQuery { pub struct TemplateListResponse { #[serde(default)] pub items: Vec, + #[serde(default)] pub pagination: Pagination, } diff --git a/crates/hypercolor-types/src/api/common.rs b/crates/hypercolor-types/src/api/common.rs index f2edf2bd6..f102cb2a8 100644 --- a/crates/hypercolor-types/src/api/common.rs +++ b/crates/hypercolor-types/src/api/common.rs @@ -4,7 +4,12 @@ use serde::{Deserialize, Serialize}; use utoipa::ToSchema; /// Pagination envelope attached to every list response. -#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize, ToSchema)] +// `Default` is the empty page, which lets list responses mark the field +// `#[serde(default)]` and keep parsing a body that omits the envelope. +// Kept out of the doc comment on purpose: utoipa publishes the doc +// comment as the schema description, so editing it moves the generated +// OpenAPI client. +#[derive(Debug, Clone, Copy, Default, PartialEq, Eq, Serialize, Deserialize, ToSchema)] pub struct Pagination { pub offset: usize, pub limit: usize, diff --git a/crates/hypercolor-types/src/api/effects.rs b/crates/hypercolor-types/src/api/effects.rs index 346dcdaa4..e04fe9dca 100644 --- a/crates/hypercolor-types/src/api/effects.rs +++ b/crates/hypercolor-types/src/api/effects.rs @@ -6,7 +6,7 @@ use serde::{Deserialize, Serialize}; use utoipa::ToSchema; use crate::api::common::Pagination; -use crate::effect::{ControlBinding, ControlDefinition, ControlValue, PresetTemplate}; +use crate::effect::{ControlDefinition, ControlValue, PresetTemplate}; /// Origin of a preset in an effect's unified preset stack. #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize, ToSchema)] @@ -215,48 +215,11 @@ pub struct UpdateActiveControlsRequest { pub controls: Option, } -/// Response for `PATCH /api/v1/effects/active/controls`. -/// -/// `effect` is the active effect's name rather than a reference object — -/// this route predates the `{ id, name }` convention its siblings use. -/// `rejected` names the controls the daemon refused, with the reason. -#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] -pub struct UpdateActiveControlsResponse { - #[serde(default)] - pub effect: String, - #[serde(default)] - pub applied: HashMap, - #[serde(default)] - pub rejected: Vec, -} - -/// Response for `PATCH /api/v1/effects/{effect_id}/controls`. -/// -/// The same body as the `active` sibling plus `controls_version`, the -/// new server-side version token. It is also returned in the `ETag` -/// header; clients echo it back via `If-Match` to get optimistic -/// concurrency on the next PATCH. -#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] -pub struct UpdateEffectControlsResponse { - #[serde(default)] - pub effect: String, - #[serde(default)] - pub applied: HashMap, - #[serde(default)] - pub rejected: Vec, - pub controls_version: u64, -} - -/// Response for `PUT /api/v1/effects/active/controls/{name}/binding`. -/// -/// `binding` is the stored binding after clamping, which can differ from -/// the one the caller sent. -#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] -pub struct SetControlBindingResponse { - pub effect: EffectRefSummary, - pub control: String, - pub binding: ControlBinding, -} +// The two control PATCH responses and the control-binding response are +// deliberately NOT defined here. Their payloads carry f32 control values, +// and the daemon builds them with `serde_json::json!`, which widens f32 to +// f64 and prints the widened digits. A derived struct writes f32 directly, +// so naming those shapes would change the bytes on the wire. /// Request body for `PUT /api/v1/effects/{id}/layout`. #[derive(Debug, Clone, Default, PartialEq, Eq, Serialize, Deserialize)] diff --git a/crates/hypercolor-types/src/api/library.rs b/crates/hypercolor-types/src/api/library.rs index 1ebbd19e5..4bbcfac0c 100644 --- a/crates/hypercolor-types/src/api/library.rs +++ b/crates/hypercolor-types/src/api/library.rs @@ -1,12 +1,8 @@ //! Library API contracts — `/api/v1/library/*`. -use std::collections::HashMap; - use serde::{Deserialize, Serialize}; use crate::api::common::Pagination; -use crate::api::effects::EffectRefSummary; -use crate::effect::ControlValue; use crate::library::{EffectPlaylist, EffectPreset}; /// Request body for `POST /api/v1/library/favorites`. @@ -34,6 +30,7 @@ pub struct FavoriteSummary { pub struct FavoriteListResponse { #[serde(default)] pub items: Vec, + #[serde(default)] pub pagination: Pagination, } @@ -114,6 +111,7 @@ pub struct ApplyPresetRequest { pub struct PresetListResponse { #[serde(default)] pub items: Vec, + #[serde(default)] pub pagination: Pagination, } @@ -127,36 +125,17 @@ pub struct DeletePresetResponse { pub deleted: bool, } -/// `{ id, name }` reference to a saved preset. -#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] -pub struct PresetRefSummary { - pub id: String, - pub name: String, -} - -/// Response for `POST /api/v1/library/presets/{id}/apply`. -/// -/// `applied_controls` is what the effect actually took, and -/// `rejected_controls` names the preset entries the effect's current -/// control definitions refused — a preset saved against an older version -/// of an effect applies partially rather than failing. -#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] -pub struct ApplyPresetResponse { - pub preset: PresetRefSummary, - pub effect: EffectRefSummary, - #[serde(default)] - pub applied_controls: HashMap, - #[serde(default)] - pub rejected_controls: Vec, - #[serde(default)] - pub warnings: Vec, -} +// The preset apply response is deliberately NOT defined here: its +// `applied_controls` map carries f32 control values, and the daemon builds +// the body with `serde_json::json!`, which widens f32 to f64 and prints the +// widened digits. Naming the shape would change the bytes on the wire. /// Response for `GET /api/v1/library/playlists`. #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] pub struct PlaylistListResponse { #[serde(default)] pub items: Vec, + #[serde(default)] pub pagination: Pagination, } diff --git a/crates/hypercolor-ui/src/api/effects.rs b/crates/hypercolor-ui/src/api/effects.rs index 80ce762e2..76997a762 100644 --- a/crates/hypercolor-ui/src/api/effects.rs +++ b/crates/hypercolor-ui/src/api/effects.rs @@ -20,11 +20,18 @@ pub use hypercolor_types::api::effects::{ ApplyEffectPresetRequest, ApplyEffectRequest as ApplyEffectBody, EffectCapabilitySet, EffectDetailResponse, EffectListResponse, EffectPresetListResponse, EffectPresetOrigin, EffectPresetSummary, EffectSummary, InstalledEffectResponse, UpdateActiveControlsRequest, - UpdateEffectControlsResponse, }; pub use hypercolor_types::api::output::{OutputPowerMode, SetOutputPowerRequest}; -/// Active effect response from `GET /api/v1/effects/active`. +/// Active effect response from `GET /api/v1/effects/active`, narrowed to +/// the running case. +/// +/// Not a mirror of the shared wire type but a projection of it: the wire +/// shape types `id` and `name` as `Option` because the idle body carries +/// nulls, and every UI consumer here has already branched on `state` and +/// wants them unwrapped. `fetch_active_effect` decodes the shared +/// `hypercolor_types::api::effects::ActiveEffectResponse` and maps idle to +/// `None`, so this type is only ever built from a decoded wire response. #[derive(Debug, Clone, Deserialize, PartialEq)] pub struct ActiveEffectResponse { pub id: String, @@ -204,6 +211,19 @@ pub enum UpdateControlsOutcome { Stale { current: u64 }, } +/// Successful control-PATCH payload — the envelope data carries the new +/// `controls_version` (also present in the `ETag` header; the body is +/// simpler to extract with `gloo_net`). +/// +/// Stays UI-local rather than moving to hypercolor-types with the rest of +/// the effects contracts: the daemon still builds that body with a +/// `serde_json::json!` literal, because naming the shape would reprint its +/// f32 control values at f32 precision and change the wire. +#[derive(Debug, Deserialize)] +struct ControlsVersionResponse { + controls_version: u64, +} + /// Scoped control PATCH against a specific effect id with optional /// optimistic-concurrency precondition. /// @@ -220,7 +240,7 @@ pub async fn update_effect_controls( let body = UpdateActiveControlsRequest { controls: Some(controls.clone()), }; - let outcome = client::send_json_versioned::<_, UpdateEffectControlsResponse>( + let outcome = client::send_json_versioned::<_, ControlsVersionResponse>( Method::PATCH, &url, Some(&body), From 49a5178f4939af33cf52b178b9896eeaf03b2e20 Mon Sep 17 00:00:00 2001 From: Stefanie Jane Date: Mon, 17 Aug 2026 12:25:22 -0700 Subject: [PATCH 5/7] refactor(api): scope the promotion to spec 78 surviving routes Spec 78's Appendix A is the normative route inventory the API converges on, and promoting a response type for a route it deletes would publish a shared contract with a scheduled death. Seven shapes come back out of hypercolor-types on that basis. Two effects responses go back to literals: /effects/stop is absent from the inventory, and /effects/active/* is deleted wholesale, which takes the controls reset with it. On the attachments side the inventory keeps /attachments/templates at GET and POST and marks the item operations deleted, so the per-template delete acknowledgement goes back to a literal, and the category and vendor facet routes are not in the inventory at all, so their four shapes return to the daemon module. What survives is what the promotion was actually worth. The attachment template catalog keeps its listing, summary, and detail, which is where the client drift fix lives. The effect-to-layout link keeps all three of its methods, rescan keeps its counts, and the library keeps favorites, presets, and playlists including the playlist stop shape, which the inventory renames to deactivate later without changing the body. Nothing here changes what the daemon writes. The seven reverted sites are byte-identical to their pre-wave form. Co-Authored-By: Nova (Claude Opus 5) --- .../hypercolor-daemon/src/api/attachments.rs | 38 +++++++++++++--- crates/hypercolor-daemon/src/api/effects.rs | 24 +++++------ .../hypercolor-types/src/api/attachments.rs | 43 ------------------- crates/hypercolor-types/src/api/effects.rs | 18 -------- 4 files changed, 44 insertions(+), 79 deletions(-) diff --git a/crates/hypercolor-daemon/src/api/attachments.rs b/crates/hypercolor-daemon/src/api/attachments.rs index 1836c9e5f..71102a71f 100644 --- a/crates/hypercolor-daemon/src/api/attachments.rs +++ b/crates/hypercolor-daemon/src/api/attachments.rs @@ -7,6 +7,7 @@ use std::sync::Arc; use axum::Json; use axum::extract::{Path as AxumPath, Query, State}; use axum::response::{IntoResponse, Response}; +use serde::Serialize; use tokio::sync::RwLockWriteGuard; use hypercolor_core::attachment::{ComponentRegistry, TemplateFilter}; @@ -24,10 +25,35 @@ use crate::domain::{DomainError, ResourceKind}; // Wire contracts live in hypercolor-types::api::attachments — shared // with the web UI and the TUI. pub use hypercolor_types::api::attachments::{ - CategoryListResponse, CategorySummary, DeleteTemplateResponse, ListTemplatesQuery, - TemplateDetail, TemplateListResponse, TemplateSummary, VendorListResponse, VendorSummary, + ListTemplatesQuery, TemplateDetail, TemplateListResponse, TemplateSummary, }; +// The category and vendor facets and the per-template item routes are not +// in spec 78's Appendix A, so their shapes stay daemon-local rather than +// entering the shared contract on the way to deletion. +#[derive(Debug, Serialize)] +pub struct CategoryListResponse { + pub items: Vec, +} + +#[derive(Debug, Serialize)] +pub struct CategorySummary { + pub category: ComponentCategory, + pub count: usize, + pub label: String, +} + +#[derive(Debug, Serialize)] +pub struct VendorListResponse { + pub items: Vec, +} + +#[derive(Debug, Serialize)] +pub struct VendorSummary { + pub vendor: String, + pub count: usize, +} + /// `GET /api/v1/attachments/templates` pub async fn list_templates( State(state): State>, @@ -162,10 +188,10 @@ pub async fn delete_template( return DomainError::Internal(anyhow::anyhow!("{error}")).into_response(); } - ApiResponse::ok(DeleteTemplateResponse { - id: removed.id, - deleted: true, - }) + ApiResponse::ok(serde_json::json!({ + "id": removed.id, + "deleted": true, + })) } /// `GET /api/v1/attachments/categories` diff --git a/crates/hypercolor-daemon/src/api/effects.rs b/crates/hypercolor-daemon/src/api/effects.rs index 58f591ac5..be43c2cae 100644 --- a/crates/hypercolor-daemon/src/api/effects.rs +++ b/crates/hypercolor-daemon/src/api/effects.rs @@ -67,8 +67,8 @@ pub use hypercolor_types::api::effects::{ EffectLayoutApplyResult, EffectLayoutResponse, EffectListResponse, EffectPresetListResponse, EffectPresetOrigin, EffectPresetSummary, EffectRefSummary, EffectSummary, InstalledEffectResponse, LayoutLinkSummary, PauseEffectResponse, RescanResponse, - ResetControlsRequest, ResetControlsResponse, ResumeEffectResponse, SetEffectLayoutResponse, - StopEffectResponse, TransitionRequest, UpdateActiveControlsRequest, + ResetControlsRequest, ResumeEffectResponse, SetEffectLayoutResponse, TransitionRequest, + UpdateActiveControlsRequest, }; struct ResolvedEffectPreset { @@ -930,10 +930,10 @@ pub async fn stop_effect(State(state): State>) -> Response { Err(error) => return error.into_response(), }; - ApiResponse::ok(StopEffectResponse { - stopped: true, - released_network_devices: stopped.released_network_devices, - }) + ApiResponse::ok(serde_json::json!({ + "stopped": true, + "released_network_devices": stopped.released_network_devices, + })) } /// `PATCH /api/v1/effects/active/controls` — Update controls on active effect @@ -1278,13 +1278,13 @@ pub async fn reset_controls( info!(effect = %effect_name, "Controls reset to defaults"); - ApiResponse::ok(ResetControlsResponse { - effect: EffectRefSummary { - id: effect_id.to_string(), - name: effect_name, + ApiResponse::ok(serde_json::json!({ + "effect": { + "id": effect_id.to_string(), + "name": effect_name, }, - reset: true, - }) + "reset": true, + })) } /// `POST /api/v1/effects/rescan` — Manually trigger an effect registry rescan. diff --git a/crates/hypercolor-types/src/api/attachments.rs b/crates/hypercolor-types/src/api/attachments.rs index 7f858f78c..482cf8b88 100644 --- a/crates/hypercolor-types/src/api/attachments.rs +++ b/crates/hypercolor-types/src/api/attachments.rs @@ -103,46 +103,3 @@ pub struct TemplateDetail { #[serde(default)] pub physical_size_mm: Option<(f32, f32)>, } - -/// Response for `DELETE /api/v1/attachments/templates/{id}`. -/// -/// Built-in templates cannot be deleted, so a success here always means -/// a user-authored template was removed. -#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] -pub struct DeleteTemplateResponse { - pub id: String, - pub deleted: bool, -} - -/// Response for `GET /api/v1/attachments/categories`. -/// -/// Unpaginated: the category set is bounded by the catalog's own -/// vocabulary rather than by how many templates are installed. -#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] -pub struct CategoryListResponse { - #[serde(default)] - pub items: Vec, -} - -/// One category and how many templates carry it. -#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] -pub struct CategorySummary { - pub category: ComponentCategory, - pub count: usize, - /// Display-ready category name, titleized for unknown categories. - pub label: String, -} - -/// Response for `GET /api/v1/attachments/vendors`. -#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] -pub struct VendorListResponse { - #[serde(default)] - pub items: Vec, -} - -/// One vendor and how many templates it has in the catalog. -#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] -pub struct VendorSummary { - pub vendor: String, - pub count: usize, -} diff --git a/crates/hypercolor-types/src/api/effects.rs b/crates/hypercolor-types/src/api/effects.rs index e04fe9dca..7dc9584ac 100644 --- a/crates/hypercolor-types/src/api/effects.rs +++ b/crates/hypercolor-types/src/api/effects.rs @@ -268,13 +268,6 @@ pub struct ResetControlsRequest { pub zone_id: Option, } -/// Response for `POST /api/v1/effects/active/reset`. -#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] -pub struct ResetControlsResponse { - pub effect: EffectRefSummary, - pub reset: bool, -} - /// `{ id, name }` reference to an effect. #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, ToSchema)] pub struct EffectRefSummary { @@ -300,17 +293,6 @@ pub struct ResumeEffectResponse { pub effect: Option, } -/// Response for `POST /api/v1/effects/stop`. -/// -/// `released_network_devices` counts the streaming network devices that -/// were handed back when the effect stopped. -#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] -pub struct StopEffectResponse { - pub stopped: bool, - #[serde(default)] - pub released_network_devices: usize, -} - /// Layout link summary in apply responses. #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, ToSchema)] pub struct LayoutLinkSummary { From f61f7251cdd727bad4c1e837fb76a46a23370a24 Mon Sep 17 00:00:00 2001 From: Stefanie Jane Date: Mon, 17 Aug 2026 13:13:29 -0700 Subject: [PATCH 6/7] refactor(types): derive Eq on the template listing as a float fence f32 is not Eq, so an Eq derive proves transitively that nothing in a type's payload is a float. That is worth having on a promoted response: the shapes these types replace were built with serde_json's json!, which widens f32 to f64 and reprints it, so a float reaching one of them is a silent wire change rather than a compile error. TemplateListResponse is the one promoted list response whose contents are Eq-capable and was missing it. The playlist and preset listings cannot have it, since the stored records they carry hold control values that really are floats; those two were already structs before this wave and never passed through json!, so they were never exposed to the reprint in the first place. Co-Authored-By: Nova (Claude Opus 5) --- crates/hypercolor-types/src/api/attachments.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/crates/hypercolor-types/src/api/attachments.rs b/crates/hypercolor-types/src/api/attachments.rs index 482cf8b88..a891c9a41 100644 --- a/crates/hypercolor-types/src/api/attachments.rs +++ b/crates/hypercolor-types/src/api/attachments.rs @@ -41,7 +41,11 @@ pub struct ListTemplatesQuery { } /// Response for `GET /api/v1/attachments/templates`. -#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] +// `Eq` is load-bearing beyond equality: f32 is not `Eq`, so deriving it +// proves transitively that nothing in this response is a float. A float +// here would be a wire hazard, because the shapes these types replace +// were built with `json!`, which widens f32 to f64 and reprints it. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] pub struct TemplateListResponse { #[serde(default)] pub items: Vec, From 5544c0c5e9c0c56ca1b415175039b313c9d63be3 Mon Sep 17 00:00:00 2001 From: Stefanie Jane Date: Mon, 17 Aug 2026 14:15:30 -0700 Subject: [PATCH 7/7] fix(api): accept a null origin and charter TemplateDetail on creation Two corrections from the codex gate on the template catalog. The promoted `origin` field rejected an explicit null. The web UI mirror it replaced declared the field as `Option`, so a body carrying `"origin": null` decoded fine there, and `serde(default)` does not cover that case: it fills in an absent key but a present null still reaches the enum and fails. Both promoted shapes now deserialize the field through a helper that maps null to the default, which is the BuiltIn the absent case already resolved to. The accepted set is now a superset of the mirror's rather than an intersection with it. Nine tests pin both directions, including that serialization still writes the value unconditionally, since the tolerance must stay deserialize-only. TemplateDetail's doc comment claimed GET, POST, and PUT. Spec 78's Appendix A keeps only the collection routes, so the item GET and PUT go away in wave 78.5 and creation is left as the sole caller. The comment now charters the type on creation and says what happens to the rest. It also records why this one type carries no Eq float fence: its physical_size_mm is a real pair of f32. That is safe because the shape was already a struct before the promotion and never passed through serde_json's json!, so it was never exposed to the f32 reprint the fence guards against. The image_url and pagination fields stay validated rather than tolerated. The mirror ignored both, so there is no prior accepted set to preserve and reading them is the capability fix this wave set out to make. Co-Authored-By: Nova (Claude Opus 5) --- .../hypercolor-types/src/api/attachments.rs | 33 +++- .../tests/api_attachments_tests.rs | 153 ++++++++++++++++++ 2 files changed, 183 insertions(+), 3 deletions(-) create mode 100644 crates/hypercolor-types/tests/api_attachments_tests.rs diff --git a/crates/hypercolor-types/src/api/attachments.rs b/crates/hypercolor-types/src/api/attachments.rs index a891c9a41..dfa72d41b 100644 --- a/crates/hypercolor-types/src/api/attachments.rs +++ b/crates/hypercolor-types/src/api/attachments.rs @@ -8,6 +8,21 @@ use crate::attachment::{ }; use crate::spatial::{LedTopology, NormalizedPosition}; +/// Accept an absent `origin`, an explicit `null`, or a real value. +/// +/// The daemon always sends a value, so this only widens what clients +/// tolerate. It exists because the hand-rolled web UI mirrors these +/// types replaced declared `origin` as an `Option` and +/// so decoded an explicit `null` happily. `#[serde(default)]` alone +/// covers the absent key but not a present null, which would make the +/// shared type stricter than the mirror it replaced. +fn origin_tolerating_null<'de, D>(deserializer: D) -> Result +where + D: serde::Deserializer<'de>, +{ + Ok(Option::::deserialize(deserializer)?.unwrap_or_default()) +} + /// Query parameters for `GET /api/v1/attachments/templates`. /// /// Every field narrows the catalog; an empty query lists everything the @@ -64,7 +79,7 @@ pub struct TemplateSummary { pub name: String, pub vendor: String, pub category: ComponentCategory, - #[serde(default)] + #[serde(default, deserialize_with = "origin_tolerating_null")] pub origin: ComponentOrigin, pub led_count: u32, pub description: String, @@ -74,18 +89,30 @@ pub struct TemplateSummary { pub tags: Vec, } -/// Response for `GET`, `POST`, and `PUT` on a single template. +/// Response for `POST /api/v1/attachments/templates`, the created +/// template. /// /// The summary's fields plus everything needed to place the attachment: /// `led_positions` is expanded from the topology at request time, so it /// is present here but never in the listing. +/// +/// The item routes that also return this body today (`GET` and `PUT` on +/// `/attachments/templates/{id}`) are deleted in wave 78.5, which leaves +/// creation as the only caller. The type is chartered on creation for +/// that reason, and the collection listing keeps its own summary shape. +// Unlike its sibling responses this one cannot carry the `Eq` float +// fence: `physical_size_mm` is a genuine `(f32, f32)`. That is safe +// here because the shape was already a struct on the daemon side before +// the promotion, so it never passed through `serde_json::json!` and was +// never exposed to the f32-to-f64 reprint the fence guards against. Its +// serialization path is unchanged. #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] pub struct TemplateDetail { pub id: String, pub name: String, pub vendor: String, pub category: ComponentCategory, - #[serde(default)] + #[serde(default, deserialize_with = "origin_tolerating_null")] pub origin: ComponentOrigin, pub led_count: u32, pub description: String, diff --git a/crates/hypercolor-types/tests/api_attachments_tests.rs b/crates/hypercolor-types/tests/api_attachments_tests.rs new file mode 100644 index 000000000..f922030f6 --- /dev/null +++ b/crates/hypercolor-types/tests/api_attachments_tests.rs @@ -0,0 +1,153 @@ +//! Attachment-template catalog contract tests. +//! +//! These pin the client-tolerance the shared types inherited from the +//! hand-rolled web UI mirrors they replaced. The mirrors declared +//! `origin` as an `Option`, so an explicit `null` +//! decoded fine; promoting the field to an unconditional +//! `ComponentOrigin` must not make that JSON stop parsing. + +use hypercolor_types::api::attachments::{TemplateDetail, TemplateListResponse, TemplateSummary}; +use hypercolor_types::attachment::{ComponentCategory, ComponentOrigin}; +use serde_json::json; + +fn summary_json(origin: Option) -> serde_json::Value { + let mut value = json!({ + "id": "ll-sl-inf", + "name": "SL Infinity", + "vendor": "Lian Li", + "category": "fan", + "led_count": 16, + "description": "120mm fan", + }); + if let Some(origin) = origin { + value["origin"] = origin; + } + value +} + +fn detail_json(origin: Option) -> serde_json::Value { + let mut value = json!({ + "id": "ll-sl-inf", + "name": "SL Infinity", + "vendor": "Lian Li", + "category": "fan", + "led_count": 2, + "description": "120mm fan", + "default_size": { "width": 0.1, "height": 0.1 }, + "topology": { + "type": "ring", + "count": 2, + "start_angle": 0.0, + "direction": "clockwise" + }, + }); + if let Some(origin) = origin { + value["origin"] = origin; + } + value +} + +// ── Deserialization tolerance ─────────────────────────────────────────── + +#[test] +fn template_summary_decodes_an_explicit_null_origin_as_built_in() { + let summary: TemplateSummary = + serde_json::from_value(summary_json(Some(serde_json::Value::Null))) + .expect("an explicit null origin must still decode"); + + assert_eq!(summary.origin, ComponentOrigin::BuiltIn); +} + +#[test] +fn template_summary_decodes_an_absent_origin_as_built_in() { + let summary: TemplateSummary = + serde_json::from_value(summary_json(None)).expect("an absent origin must still decode"); + + assert_eq!(summary.origin, ComponentOrigin::BuiltIn); +} + +#[test] +fn template_summary_keeps_an_explicit_origin() { + let summary: TemplateSummary = serde_json::from_value(summary_json(Some(json!("user")))) + .expect("an explicit origin must decode"); + + assert_eq!(summary.origin, ComponentOrigin::User); +} + +#[test] +fn template_detail_decodes_an_explicit_null_origin_as_built_in() { + let detail: TemplateDetail = serde_json::from_value(detail_json(Some(serde_json::Value::Null))) + .expect("an explicit null origin must still decode"); + + assert_eq!(detail.origin, ComponentOrigin::BuiltIn); +} + +#[test] +fn template_detail_decodes_an_absent_origin_as_built_in() { + let detail: TemplateDetail = + serde_json::from_value(detail_json(None)).expect("an absent origin must still decode"); + + assert_eq!(detail.origin, ComponentOrigin::BuiltIn); +} + +#[test] +fn template_detail_keeps_an_explicit_origin() { + let detail: TemplateDetail = + serde_json::from_value(detail_json(Some(json!("user")))).expect("origin must decode"); + + assert_eq!(detail.origin, ComponentOrigin::User); +} + +// ── Serialization is unchanged ────────────────────────────────────────── + +#[test] +fn template_summary_always_serializes_origin() { + // The tolerance above is deserialize-only. The daemon still writes + // the key unconditionally, including for the default variant, so a + // `skip_serializing_if` creeping in would be a wire change. + let summary = TemplateSummary { + id: "ll-sl-inf".to_owned(), + name: "SL Infinity".to_owned(), + vendor: "Lian Li".to_owned(), + category: ComponentCategory::Fan, + origin: ComponentOrigin::BuiltIn, + led_count: 16, + description: "120mm fan".to_owned(), + image_url: None, + tags: Vec::new(), + }; + + let value = serde_json::to_value(&summary).expect("summary must serialize"); + + assert_eq!(value["origin"], json!("built_in")); + // `image_url` is likewise emitted as an explicit null rather than + // dropped, which is what the pre-promotion daemon struct did. + assert_eq!(value["image_url"], serde_json::Value::Null); +} + +#[test] +fn template_summary_round_trips_through_a_null_origin() { + let decoded: TemplateSummary = + serde_json::from_value(summary_json(Some(serde_json::Value::Null))) + .expect("null origin decodes"); + let reencoded = serde_json::to_value(&decoded).expect("summary must serialize"); + + // Re-encoding resolves the null to the concrete default, which is + // the shape the daemon would have sent in the first place. + assert_eq!(reencoded["origin"], json!("built_in")); + + let again: TemplateSummary = serde_json::from_value(reencoded).expect("re-decode"); + assert_eq!(again, decoded); +} + +#[test] +fn template_listing_tolerates_a_missing_pagination_envelope() { + // The deleted UI mirror had no `pagination` field at all and simply + // ignored the envelope, so a body without one must still parse. + let listing: TemplateListResponse = + serde_json::from_value(json!({ "items": [summary_json(None)] })) + .expect("a listing without pagination must decode"); + + assert_eq!(listing.items.len(), 1); + assert_eq!(listing.pagination.total, 0); +}