From 86c6d7ec24c024e179c54cc45c015bb0a7dd6ed5 Mon Sep 17 00:00:00 2001 From: Cameron Reeves Date: Thu, 27 Aug 2026 23:50:42 +1000 Subject: [PATCH 01/14] feat(signage-ai): image generation runtime and controllers Generate and edit signage artwork through a domain's own vendor account. A request validates, reserves a slot per vendor call, writes a job row and hands off to a fiber, answering 202. The client long polls jobs/:id, which holds until the version moves or the wait runs out, so a candidate reaches the browser about half a second after it lands without needing a socket. - ImageGen::Adapter with OpenAI (and Azure OpenAI, same wire shape) and Gemini on Vertex. Vertex only for Google: an AI Studio key carries neither the indemnity nor the no training terms. - Slots caps concurrent vendor calls per replica. A request that cannot reserve every call it needs is told the service is busy and no row is written, so there is never a queued job nobody is working on. - Store writes results through the same Storage and Upload machinery the uploads controller uses, from outside a request. - Prompt asks for a clear headline area and no lettering, because the words and the logo are composited by the browser. - Sweep removes candidates nobody kept after the retention window and fails jobs left running by a replica that went away. Co-Authored-By: Claude Opus 5 (1M context) --- src/app.cr | 4 + src/constants.cr | 23 + .../controllers/application.cr | 25 + .../controllers/signage/ai.cr | 585 ++++++++++++++++++ .../controllers/signage/ai_providers.cr | 192 ++++++ src/placeos-rest-api/error.cr | 47 ++ src/placeos-rest-api/utilities/image_gen.cr | 52 ++ .../utilities/image_gen/adapter.cr | 75 +++ .../image_gen/adapters/gemini_vertex.cr | 207 +++++++ .../image_gen/adapters/openai_images.cr | 193 ++++++ .../utilities/image_gen/http.cr | 85 +++ .../utilities/image_gen/prompt.cr | 117 ++++ .../utilities/image_gen/runner.cr | 206 ++++++ .../utilities/image_gen/slots.cr | 39 ++ .../utilities/image_gen/store.cr | 113 ++++ .../utilities/image_gen/sweep.cr | 106 ++++ .../utilities/image_gen/types.cr | 79 +++ 17 files changed, 2148 insertions(+) create mode 100644 src/placeos-rest-api/controllers/signage/ai.cr create mode 100644 src/placeos-rest-api/controllers/signage/ai_providers.cr create mode 100644 src/placeos-rest-api/utilities/image_gen.cr create mode 100644 src/placeos-rest-api/utilities/image_gen/adapter.cr create mode 100644 src/placeos-rest-api/utilities/image_gen/adapters/gemini_vertex.cr create mode 100644 src/placeos-rest-api/utilities/image_gen/adapters/openai_images.cr create mode 100644 src/placeos-rest-api/utilities/image_gen/http.cr create mode 100644 src/placeos-rest-api/utilities/image_gen/prompt.cr create mode 100644 src/placeos-rest-api/utilities/image_gen/runner.cr create mode 100644 src/placeos-rest-api/utilities/image_gen/slots.cr create mode 100644 src/placeos-rest-api/utilities/image_gen/store.cr create mode 100644 src/placeos-rest-api/utilities/image_gen/sweep.cr create mode 100644 src/placeos-rest-api/utilities/image_gen/types.cr diff --git a/src/app.cr b/src/app.cr index 63f0eca7..13b5cda4 100644 --- a/src/app.cr +++ b/src/app.cr @@ -108,6 +108,10 @@ Signal::INT.trap &terminate # Docker containers use the term signal Signal::TERM.trap &terminate +# Housekeeping for generated signage artwork: candidates nobody kept, and jobs +# left running by a replica that went away. +PlaceOS::Api::ImageGen::Sweep.start + # Start the server server.run do PlaceOS::Api::Log.info { "listening on #{server.print_addresses}" } diff --git a/src/constants.cr b/src/constants.cr index 1e3c67c9..a928cdff 100644 --- a/src/constants.cr +++ b/src/constants.cr @@ -22,6 +22,29 @@ module PlaceOS::Api # https://developer.mozilla.org/en-US/docs/Web/API/RTCIceServer WEBRTC_DEFAULT_ICE_CONFIG = ENV["WEBRTC_DEFAULT_ICE_CONFIG"]? || {urls: "stun:stun.l.google.com:19302"}.to_json + # Signage AI image generation + #################################################################################################### + + # concurrent vendor image calls per replica; a request that cannot reserve a slot + # for every candidate is told the service is busy rather than queued + SIGNAGE_AI_MAX_CALLS = (ENV["SIGNAGE_AI_MAX_CALLS"]? || "6").to_i + + # how long to wait on a single vendor call + SIGNAGE_AI_READ_TIMEOUT = (ENV["SIGNAGE_AI_READ_TIMEOUT"]? || "180").to_i.seconds + + # unclaimed candidates and references are swept after this long + SIGNAGE_AI_RETENTION = (ENV["SIGNAGE_AI_RETENTION_HOURS"]? || "48").to_i.hours + + # a job still running after this is treated as abandoned by a departed replica + SIGNAGE_AI_JOB_STALE = (ENV["SIGNAGE_AI_JOB_STALE_MINUTES"]? || "10").to_i.minutes + + # kill switch: capabilities reports disabled and generate returns 503 + SIGNAGE_AI_DISABLED = ENV["SIGNAGE_AI_DISABLED"]?.try(&.downcase) == "true" + + # default quotas, overridden per provider row + SIGNAGE_AI_USER_PER_DAY = (ENV["SIGNAGE_AI_USER_PER_DAY"]? || "60").to_i + SIGNAGE_AI_DOMAIN_PER_MONTH = (ENV["SIGNAGE_AI_DOMAIN_PER_MONTH"]? || "2000").to_i + # server defaults in `./app.cr` TRIGGERS_URI = URI.parse(ENV["TRIGGERS_URI"]? || "http://triggers:3000") diff --git a/src/placeos-rest-api/controllers/application.cr b/src/placeos-rest-api/controllers/application.cr index 84560c07..0a9b815b 100644 --- a/src/placeos-rest-api/controllers/application.cr +++ b/src/placeos-rest-api/controllers/application.cr @@ -249,6 +249,31 @@ module PlaceOS::Api CommonError.new(error, false) end + # Signage AI image generation. `kind` lets a client branch without matching + # on message text. + struct ImageGenError + include JSON::Serializable + include YAML::Serializable + + getter error : String + getter kind : String + + def initialize(@error, @kind) + end + end + + @[AC::Route::Exception(Error::ImageGen::Quota, status_code: HTTP::Status::TOO_MANY_REQUESTS)] + @[AC::Route::Exception(Error::ImageGen::Moderated, status_code: HTTP::Status::UNPROCESSABLE_ENTITY)] + @[AC::Route::Exception(Error::ImageGen::Busy, status_code: HTTP::Status::SERVICE_UNAVAILABLE)] + @[AC::Route::Exception(Error::ImageGen::NotConfigured, status_code: HTTP::Status::SERVICE_UNAVAILABLE)] + @[AC::Route::Exception(Error::ImageGen::Vendor, status_code: HTTP::Status::BAD_GATEWAY)] + @[AC::Route::Exception(Error::ImageGen::Timeout, status_code: HTTP::Status::GATEWAY_TIMEOUT)] + @[AC::Route::Exception(Error::ImageGen::Permission, status_code: HTTP::Status::FORBIDDEN)] + def image_generation_failed(error) : ImageGenError + Log.debug(exception: error) { error.message } + ImageGenError.new(error.message || "image generation failed", error.kind) + end + # 406 when a request cannot be satisfied (e.g. no approvers available) @[AC::Route::Exception(Error::NotAcceptable, status_code: HTTP::Status::NOT_ACCEPTABLE)] def resource_not_acceptable(error) : CommonError diff --git a/src/placeos-rest-api/controllers/signage/ai.cr b/src/placeos-rest-api/controllers/signage/ai.cr new file mode 100644 index 00000000..404c3808 --- /dev/null +++ b/src/placeos-rest-api/controllers/signage/ai.cr @@ -0,0 +1,585 @@ +require "placeos-models/metadata" +require "placeos-models/playlist/item" +require "placeos-models/signage_ai_job" +require "placeos-models/signage_ai_provider" +require "placeos-models/storage" +require "placeos-models/upload" + +require "../application" + +module PlaceOS::Api + # Generate and edit signage artwork. + # + # A request validates, reserves a slot per vendor call, writes a job row and + # hands off to a fiber, answering 202 immediately. The client then long polls + # `jobs/:id`, which holds the connection until something changes or the wait + # runs out, so a finished candidate reaches the browser within about half a + # second of landing without a socket. + class SignageAI < Application + include Utils::GroupPermissions + + base "/api/engine/v2/signage/ai" + + # Scopes + ############################################################################################### + + before_action :can_read, only: [:capabilities, :show_job, :index_jobs, :usage] + before_action :can_write, only: [:generate, :edit, :cancel, :claim] + + ############################################################################################### + + getter authority : ::PlaceOS::Model::Authority { current_authority.as(::PlaceOS::Model::Authority) } + + @[AC::Route::Filter(:before_action, only: [:show_job, :cancel, :claim])] + def find_current_job(id : UUID) + Log.context.set(signage_ai_job: id.to_s) + job = ::PlaceOS::Model::SignageAIJob.find!(id) + raise Error::NotFound.new("no such job") unless job.authority_id == authority.id + @current_job = job + end + + getter! current_job : ::PlaceOS::Model::SignageAIJob + + # Requests + ############################################################################################### + + struct GenerateParams + include JSON::Serializable + + getter prompt : String = "" + getter aspect_ratio : String = "16:9" + getter quality : String = "standard" + getter candidates : Int32 = 2 + getter references : Array(String) = [] of String + getter include_logo : Bool = true + getter add_text_with_layer : Bool = true + getter words : String? = nil + getter provider_id : UUID? = nil + getter model : String? = nil + getter group_id : UUID? = nil + getter idempotency_key : String? = nil + end + + struct EditParams + include JSON::Serializable + + getter prompt : String = "" + getter aspect_ratio : String = "16:9" + getter quality : String = "standard" + getter candidates : Int32 = 1 + getter references : Array(String) = [] of String + getter include_logo : Bool = true + getter add_text_with_layer : Bool = true + getter words : String? = nil + getter provider_id : UUID? = nil + getter model : String? = nil + getter group_id : UUID? = nil + getter idempotency_key : String? = nil + + getter source_upload_id : String = "" + getter source_item_id : String? = nil + getter parent_job_id : UUID? = nil + end + + struct ClaimParams + include JSON::Serializable + + getter upload_id : String = "" + getter item_id : String = "" + end + + # Responses + ############################################################################################### + + struct Capabilities + include JSON::Serializable + + getter enabled : Bool + getter reason : String? + getter providers : Array(ImageGen::ProviderCapabilities) + getter default_provider_id : String? + getter aspect_ratios : Array(String) + getter qualities : Array(String) + getter max_candidates : Int32 + getter logo_layer : Bool + getter quota : NamedTuple(user_remaining_today: Int32?, domain_remaining_month: Int32?) + + def initialize(@enabled, @providers, @default_provider_id, @quota, + @logo_layer = false, @reason = nil, + @aspect_ratios = ImageGen::ASPECTS, + @qualities = ImageGen::QUALITIES, + @max_candidates = ImageGen::MAX_CANDIDATES) + end + end + + struct JobResponse + include JSON::Serializable + + getter id : String + getter state : String + getter kind : String + getter provider : String? + getter model : String? + getter candidates : Int32 + getter images_produced : Int32 + getter parent_job_id : String? + getter version : Int32 + getter prompt : String? + getter images : Array(JSON::Any) + getter error_kind : String? + getter error_message : String? + getter cost_units : Float64? + getter latency_ms : Int64? + getter created_at : Int64? + getter finished_at : Int64? + + def initialize(job : ::PlaceOS::Model::SignageAIJob) + @id = job.id.to_s + @state = job.state.to_s + @kind = job.kind.to_s + @provider = job.provider_type + @model = job.model + @candidates = job.candidates + @images_produced = job.images_produced + @parent_job_id = job.parent_job_id.try(&.to_s) + @version = job.version + @prompt = job.prompt + @images = job.images + @error_kind = job.error_kind + @error_message = job.error_message + @cost_units = job.cost_units + @latency_ms = job.latency_ms + @created_at = job.created_at.try(&.to_unix) + @finished_at = job.finished_at.try(&.to_unix) + end + end + + # Routes + ############################################################################################### + + # what this domain can do, and what the caller has left of their quota + @[AC::Route::GET("/capabilities")] + def capabilities : Capabilities + return disabled("the feature is switched off") if SIGNAGE_AI_DISABLED + + rows = ::PlaceOS::Model::SignageAIProvider.available_for(authority.id) + return disabled("no AI provider is configured for this domain") if rows.empty? + + begin + ::PlaceOS::Model::Storage.storage_or_default(authority.id) + rescue + return disabled("no upload storage is configured for this domain") + end + + default = ::PlaceOS::Model::SignageAIProvider.default_for(authority.id) + + Capabilities.new( + enabled: true, + providers: rows.map { |row| ImageGen::Adapter.for(row).capabilities }, + default_provider_id: default.try(&.id.to_s), + logo_layer: !brand_kit.try(&.logo_upload_id).nil?, + quota: remaining_quota(default), + ) + end + + # start a generate + @[AC::Route::POST("/generate", body: :params, status_code: HTTP::Status::ACCEPTED)] + def generate(params : GenerateParams) : JobResponse + raise Error::ImageGen::NotConfigured.new("the feature is switched off") if SIGNAGE_AI_DISABLED + raise Error::ModelValidation.new([Error::Field.new(:prompt, "is required")]) if params.prompt.blank? + + start_job( + kind: ImageGen::Kind::Generate, + prompt: params.prompt, + aspect: params.aspect_ratio, + quality: params.quality, + candidates: params.candidates, + references: params.references, + include_logo: params.include_logo, + text_layer: params.add_text_with_layer, + words: params.words, + provider_id: params.provider_id, + model: params.model, + group_id: params.group_id, + idempotency_key: params.idempotency_key, + ) + end + + # start an edit, or a refine of an earlier job + @[AC::Route::POST("/edit", body: :params, status_code: HTTP::Status::ACCEPTED)] + def edit(params : EditParams) : JobResponse + raise Error::ImageGen::NotConfigured.new("the feature is switched off") if SIGNAGE_AI_DISABLED + raise Error::ModelValidation.new([Error::Field.new(:prompt, "is required")]) if params.prompt.blank? + raise Error::ModelValidation.new([Error::Field.new(:source_upload_id, "is required")]) if params.source_upload_id.blank? + + source = readable_upload(params.source_upload_id, params.source_item_id) + + parent = if (parent_id = params.parent_job_id) + job = ::PlaceOS::Model::SignageAIJob.find?(parent_id) + raise Error::NotFound.new("no such parent job") if job.nil? || job.authority_id != authority.id + job + end + + start_job( + kind: ImageGen::Kind::Edit, + prompt: params.prompt, + aspect: params.aspect_ratio, + quality: params.quality, + candidates: params.candidates, + references: params.references, + include_logo: params.include_logo, + text_layer: params.add_text_with_layer, + words: params.words, + provider_id: params.provider_id, + model: params.model, + group_id: params.group_id, + idempotency_key: params.idempotency_key, + source: source, + parent: parent, + ) + end + + # long poll: holds until the job changes past `since`, or `wait` runs out + @[AC::Route::GET("/jobs/:id")] + def show_job( + @[AC::Param::Info(description: "seconds to hold the request open, 0 to answer immediately", example: "25")] + wait : Int32 = 0, + @[AC::Param::Info(description: "the version the caller already has", example: "3")] + since : Int32? = nil, + ) : JobResponse + job = current_job + return JobResponse.new(job) if wait <= 0 || job.final? + + deadline = Time.utc + Math.min(wait, MAX_WAIT).seconds + known = since || job.version + + while Time.utc < deadline + sleep POLL_INTERVAL + fresh = on_primary { ::PlaceOS::Model::SignageAIJob.find?(job.id.as(UUID)) } + next if fresh.nil? + job = fresh + break if job.version > known || job.final? + end + + JobResponse.new(job) + end + + # the caller's recent jobs, for the recent generations list + @[AC::Route::GET("/jobs")] + def index_jobs( + @[AC::Param::Info(description: "only the caller's own jobs", example: "true")] + mine : Bool = true, + limit : Int32 = 20, + ) : Array(JobResponse) + limit = limit.clamp(1, 100) + + query = ::PlaceOS::Model::SignageAIJob.where(authority_id: authority.id.as(String)) + if mine + query = query.where(user_id: current_user.id.as(String)) + else + check_support + end + + query + .where("created_at > ?", 7.days.ago) + .order(created_at: :desc) + .limit(limit) + .to_a + .map { |job| JobResponse.new(job) } + end + + # ask a running job to stop. Calls already with a vendor run to completion. + @[AC::Route::POST("/jobs/:id/cancel")] + def cancel : JobResponse + job = current_job + raise Error::Forbidden.new unless job.user_id == current_user.id || user_support? + + unless job.final? + job.cancel_requested = true + job.version = job.version + 1 + job.save + end + + JobResponse.new(job) + end + + # record that a candidate became a media item, so the sweep leaves it alone + @[AC::Route::POST("/jobs/:id/claim", body: :params)] + def claim(params : ClaimParams) : JobResponse + job = current_job + raise Error::Forbidden.new unless job.user_id == current_user.id || user_support? + + item = ::PlaceOS::Model::Playlist::Item.find!(params.item_id) + raise Error::Forbidden.new("item belongs to another domain") unless item.authority_id == authority.id + raise Error::ModelValidation.new([Error::Field.new(:item_id, "does not use this image")]) unless item.media_id == params.upload_id + + index = job.images.index { |image| image["upload_id"]?.try(&.as_s?) == params.upload_id } + raise Error::NotFound.new("that image is not part of this job") if index.nil? + + upload = ::PlaceOS::Model::Upload.find?(params.upload_id) + if upload + tags = upload.tags.reject { |tag| tag == ImageGen::Store::CANDIDATE_TAG } + tags << "ai-claimed" unless tags.includes?("ai-claimed") + upload.tags = tags + upload.save + end + + entry = job.images[index].as_h + entry["item_id"] = JSON::Any.new(item.id.as(String)) + ::PlaceOS::Model::SignageAIJob.bump_image(job.id.as(UUID), index, entry) + + JobResponse.new(::PlaceOS::Model::SignageAIJob.find!(job.id.as(UUID))) + end + + # spend per provider and model, for the Backoffice usage tab + @[AC::Route::GET("/usage")] + def usage( + @[AC::Param::Info(description: "unix seconds, defaults to 30 days ago")] + from : Int64? = nil, + @[AC::Param::Info(description: "unix seconds, defaults to now")] + to : Int64? = nil, + ) : Array(::PlaceOS::Model::SignageAIJob::UsageRow) + check_support + + start = from ? Time.unix(from) : 30.days.ago + finish = to ? Time.unix(to) : Time.utc + ::PlaceOS::Model::SignageAIJob.usage(authority.id.as(String), start, finish) + end + + # Internals + ############################################################################################### + + MAX_WAIT = 25 + POLL_INTERVAL = 500.milliseconds + + private def disabled(reason : String) : Capabilities + Capabilities.new( + enabled: false, + providers: [] of ImageGen::ProviderCapabilities, + default_provider_id: nil, + quota: {user_remaining_today: nil, domain_remaining_month: nil}, + reason: reason, + ) + end + + private def brand_kit : ImageGen::Prompt::BrandKit? + zone_id = support_org_zone_id + return nil unless zone_id + metadata = ::PlaceOS::Model::Metadata.build_metadata(zone_id, "signage_ai")["signage_ai"]? + ImageGen::Prompt::BrandKit.parse(metadata.try(&.details)) + rescue ex + Log.warn(exception: ex) { "could not read the signage_ai brand kit" } + nil + end + + private def quotas_for(row : ::PlaceOS::Model::SignageAIProvider?) : Tuple(Int32, Int32) + user = row.try(&.quota("user_per_day")) || SIGNAGE_AI_USER_PER_DAY + domain = row.try(&.quota("domain_per_month")) || SIGNAGE_AI_DOMAIN_PER_MONTH + {user, domain} + end + + private def remaining_quota(row : ::PlaceOS::Model::SignageAIProvider?) + user_limit, domain_limit = quotas_for(row) + used_today = ::PlaceOS::Model::SignageAIJob.sum_candidates(current_user.id.as(String), 1.day.ago) + used_month = ::PlaceOS::Model::SignageAIJob.sum_candidates_for_authority(authority.id.as(String), 30.days.ago) + + { + user_remaining_today: Math.max(0, user_limit - used_today), + domain_remaining_month: Math.max(0, domain_limit - used_month), + } + end + + # An upload the caller may use as a source or a reference: one they own, or + # one behind a media item in this domain they can read. Uploads carry no + # authority of their own, so an item is how a shared image is proved. + private def readable_upload(upload_id : String, item_id : String? = nil) : ::PlaceOS::Model::Upload + upload = ::PlaceOS::Model::Upload.find?(upload_id) + raise Error::NotFound.new("no such upload") if upload.nil? + + return upload if upload.uploaded_by == current_user.id + return upload if user_support? + + raise Error::ImageGen::Permission.new("this image needs the media item it belongs to") if item_id.nil? + + item = ::PlaceOS::Model::Playlist::Item.find?(item_id) + raise Error::ImageGen::Permission.new("no such media item") if item.nil? + raise Error::ImageGen::Permission.new("that item belongs to another domain") unless item.authority_id == authority.id + unless item.media_id == upload_id || item.thumbnail_id == upload_id + raise Error::ImageGen::Permission.new("that item does not use this image") + end + + groups = ::PlaceOS::Model::GroupPlaylistItem.where(playlist_item_id: item.id).to_a.map(&.group_id) + raise Error::ImageGen::Permission.new("that item is not shared with you") if groups.empty? + + permissions = effective_permissions_for(current_user, groups) + raise Error::ImageGen::Permission.new("that item is not shared with you") unless permissions.read? + + upload + end + + # The group a non-admin caller is acting in: they need Create on it, and it + # has to be a signage group, the same test sharing uses. + private def check_create_permission(group_id : UUID?) : Nil + return if user_support? + + raise Error::Forbidden.new("group_id required") if group_id.nil? + + group = ::PlaceOS::Model::Group.find?(group_id) + raise Error::Forbidden.new("no such group") if group.nil? + raise Error::Forbidden.new("group must be in the same authority") unless group.authority_id == authority.id + raise Error::Forbidden.new("group must participate in the 'signage' subsystem") unless group.subsystems.includes?("signage") + + permissions = group_memberships(current_user)[group_id]? || ::PlaceOS::Model::Permissions::None + raise Error::Forbidden.new("missing Create permission on the target group") unless permissions.create? + end + + private def start_job( + kind : ImageGen::Kind, + prompt : String, + aspect : String, + quality : String, + candidates : Int32, + references : Array(String), + include_logo : Bool, + text_layer : Bool, + words : String?, + provider_id : UUID?, + model : String?, + group_id : UUID?, + idempotency_key : String?, + source : ::PlaceOS::Model::Upload? = nil, + parent : ::PlaceOS::Model::SignageAIJob? = nil, + ) : JobResponse + raise Error::ModelValidation.new([Error::Field.new(:aspect_ratio, "must be one of #{ImageGen::ASPECTS.join(", ")}")]) unless ImageGen.aspect_valid?(aspect) + candidates = candidates.clamp(1, ImageGen::MAX_CANDIDATES) + + check_create_permission(group_id) + + # a repeat of a submission already accepted returns the same job rather + # than spending again + if (key = idempotency_key.presence) + existing = on_primary do + ::PlaceOS::Model::SignageAIJob.where(user_id: current_user.id.as(String), idempotency_key: key).first? + end + return JobResponse.new(existing) if existing + end + + row = if provider_id + found = ::PlaceOS::Model::SignageAIProvider.find?(provider_id) + raise Error::NotFound.new("no such provider") if found.nil? + unless found.authority_id.nil? || found.authority_id == authority.id + raise Error::Forbidden.new("that provider belongs to another domain") + end + found + else + ::PlaceOS::Model::SignageAIProvider.default_for(authority.id) + end + raise Error::ImageGen::NotConfigured.new("no AI provider is configured for this domain") if row.nil? + raise Error::ImageGen::NotConfigured.new("that provider is switched off") unless row.enabled + + storage = begin + ::PlaceOS::Model::Storage.storage_or_default(authority.id) + rescue ex + raise Error::ImageGen::NotConfigured.new("no upload storage is configured for this domain") + end + + user_limit, domain_limit = quotas_for(row) + used_today = ::PlaceOS::Model::SignageAIJob.sum_candidates(current_user.id.as(String), 1.day.ago) + raise Error::ImageGen::Quota.new("you have used your image allowance for today") if used_today + candidates > user_limit + used_month = ::PlaceOS::Model::SignageAIJob.sum_candidates_for_authority(authority.id.as(String), 30.days.ago) + raise Error::ImageGen::Quota.new("this domain has used its image allowance for the month") if used_month + candidates > domain_limit + + adapter = ImageGen::Adapter.for(row) + chosen_model = model || row.default_model || adapter.capabilities.default_model + raise Error::ImageGen::NotConfigured.new("no model configured for #{row.name}") if chosen_model.nil? + + allowed = adapter.capabilities.models.map(&.id) + raise Error::ModelValidation.new([Error::Field.new(:model, "is not available on this provider")]) unless allowed.includes?(chosen_model) + + reference_ids = references.first(8).map { |id| readable_upload(id).id.as(String) } + if include_logo && (logo = brand_kit.try(&.logo_upload_id).presence) + reference_ids << logo unless reference_ids.includes?(logo) + end + + history = parent ? (parent.chain.compact_map(&.prompt) + [parent.prompt].compact) : [] of String + + text = ImageGen::Prompt.build(ImageGen::Prompt::Options.new( + brief: parent ? (history.first? || prompt) : prompt, + aspect: aspect, + text_mode: text_layer ? ImageGen::Prompt::TextMode::Layer : ImageGen::Prompt::TextMode::Model, + include_logo: include_logo, + brand: brand_kit, + words: words, + history: parent ? history[1..]? || [] of String : [] of String, + instruction: parent ? prompt : nil, + )) + + request = ImageGen::AdapterRequest.new( + kind: kind, + prompt: text, + aspect: aspect, + quality: quality, + candidates: candidates, + model: chosen_model, + ) + + # every vendor call needs a slot, taken before anything is written, so a + # busy replica never leaves a job nobody is working on + calls = adapter.calls_for(candidates) + unless ImageGen.slots.try_reserve(calls) + raise Error::ImageGen::Busy.new("too many images are being generated right now, try again in a moment") + end + + job = ::PlaceOS::Model::SignageAIJob.new( + authority_id: authority.id.as(String), + provider_id: row.id.as(UUID), + provider_type: row.provider.to_s, + model: chosen_model, + user_id: current_user.id, + user_email: current_user.email.to_s, + user_name: current_user.name, + parent_job_id: parent.try(&.id.as(UUID)), + kind: kind == ImageGen::Kind::Edit ? ::PlaceOS::Model::SignageAIJob::Kind::Edit : ::PlaceOS::Model::SignageAIJob::Kind::Generate, + candidates: candidates, + idempotency_key: idempotency_key.presence, + prompt: prompt, + ) + job.request = JSON::Any.new({ + "aspect_ratio" => JSON::Any.new(aspect), + "quality" => JSON::Any.new(quality), + "include_logo" => JSON::Any.new(include_logo), + "text_layer" => JSON::Any.new(text_layer), + "references" => JSON::Any.new(reference_ids.map { |id| JSON::Any.new(id) }), + }) + job.result = JSON::Any.new({ + "images" => JSON::Any.new(Array(JSON::Any).new(candidates) { JSON::Any.new(nil) }), + }) + job.upload_ids = reference_ids + + unless job.save + ImageGen.slots.release(calls) + raise Error::ModelValidation.new(job.errors) + end + + context = ImageGen::Runner::Context.new( + job_id: job.id.as(UUID), + authority_id: authority.id.as(String), + hostname: request_hostname, + user: current_user, + storage: storage, + adapter: adapter, + request: request, + source_upload_id: source.try(&.id.as(String)), + reference_upload_ids: reference_ids, + ) + + spawn { ImageGen::Runner.run(context) } + + JobResponse.new(job) + end + + private def request_hostname : String + request.hostname.presence || authority.domain + end + end +end diff --git a/src/placeos-rest-api/controllers/signage/ai_providers.cr b/src/placeos-rest-api/controllers/signage/ai_providers.cr new file mode 100644 index 00000000..ffc45369 --- /dev/null +++ b/src/placeos-rest-api/controllers/signage/ai_providers.cr @@ -0,0 +1,192 @@ +require "placeos-models/signage_ai_provider" + +require "../application" + +module PlaceOS::Api + # Vendor credentials for signage image generation. + # + # The Data Stores rule: sys_admin writes, support reads. Responses never carry + # the credentials, so every route renders `as_json` rather than the row. + class SignageAIProviders < Application + base "/api/engine/v2/signage/ai/providers" + + # Scopes + ############################################################################################### + + before_action :can_read, only: [:index, :show] + before_action :can_write, only: [:create, :update, :destroy, :test] + + before_action :check_admin, only: [:create, :update, :destroy, :test] + before_action :check_support, only: [:index, :show] + + ############################################################################################### + + @[AC::Route::Filter(:before_action, except: [:index, :create])] + def find_current_provider(id : UUID) + Log.context.set(signage_ai_provider: id.to_s) + @current_provider = ::PlaceOS::Model::SignageAIProvider.find!(id) + end + + getter! current_provider : ::PlaceOS::Model::SignageAIProvider + + # What a client may send. `credentials` is a JSON object whose shape depends + # on the provider; it is required on create and, left out on update, the + # stored value is kept. + struct ProviderParams + include JSON::Serializable + + getter name : String? = nil + getter provider : String? = nil + getter authority_id : String? = nil + getter credentials : JSON::Any? = nil + getter endpoint : String? = nil + getter location : String? = nil + getter default_model : String? = nil + getter allowed_models : Array(String)? = nil + getter enabled : Bool? = nil + getter is_default : Bool? = nil + getter quotas : JSON::Any? = nil + end + + alias ProviderJSON = NamedTuple( + id: String, + name: String, + provider: String, + authority_id: String?, + endpoint: String?, + location: String?, + default_model: String?, + allowed_models: Array(String), + enabled: Bool, + is_default: Bool, + quotas: JSON::Any, + created_at: Int64, + updated_at: Int64) + + # rows for a domain, or every row when no domain is given + @[AC::Route::GET("/")] + def index( + @[AC::Param::Info(description: "return the rows belonging to this authority", example: "authority-1234")] + authority_id : String? = nil, + @[AC::Param::Info(description: "include the shared fallback row", example: "true")] + include_shared : Bool = true, + ) : Array(ProviderJSON) + rows = if authority_id + if include_shared + ::PlaceOS::Model::SignageAIProvider.available_for(authority_id) + else + ::PlaceOS::Model::SignageAIProvider.where(authority_id: authority_id).to_a + end + else + ::PlaceOS::Model::SignageAIProvider.all.to_a + end + + rows.map { |row| row.as_json } + end + + @[AC::Route::GET("/:id")] + def show : ProviderJSON + current_provider.as_json + end + + @[AC::Route::POST("/", body: :params, status_code: HTTP::Status::CREATED)] + def create(params : ProviderParams) : ProviderJSON + row = ::PlaceOS::Model::SignageAIProvider.new + + name = params.name + raise Error::ModelValidation.new([Error::Field.new(:name, "is required")]) if name.nil? || name.empty? + row.name = name + + row.provider = parse_provider(params.provider) + + credentials = params.credentials + if credentials.nil? || credentials.as_h?.nil? + raise Error::ModelValidation.new([Error::Field.new(:credentials, "must be a JSON object")]) + end + row.credentials = credentials.to_json + + row.authority_id = params.authority_id + apply_optional(row, params) + + raise Error::ModelValidation.new(row.errors) unless row.save + row.as_json + end + + @[AC::Route::PATCH("/:id", body: :params)] + @[AC::Route::PUT("/:id", body: :params)] + def update(params : ProviderParams) : ProviderJSON + row = current_provider + + row.name = params.name.not_nil! if params.name.presence + row.provider = parse_provider(params.provider) if params.provider.presence + + # left out (or blank) means keep what is stored + if (credentials = params.credentials) && (hash = credentials.as_h?) && !hash.empty? + row.credentials = credentials.to_json + end + + apply_optional(row, params) + + raise Error::ModelValidation.new(row.errors) unless row.save + row.as_json + end + + @[AC::Route::DELETE("/:id", status_code: HTTP::Status::ACCEPTED)] + def destroy : Nil + current_provider.destroy + end + + record TestResult, ok : Bool, latency_ms : Int64, model : String?, error : String? = nil, kind : String? = nil do + include JSON::Serializable + end + + # Prove a row's credentials work, without leaving anything behind: one small + # image, discarded. + @[AC::Route::POST("/:id/test")] + def test : TestResult + row = current_provider + adapter = ImageGen::Adapter.for(row) + model = row.default_model || adapter.capabilities.default_model + + started = Time.utc + begin + raise Error::ImageGen::NotConfigured.new("no model configured") if model.nil? + + adapter.generate(ImageGen::AdapterRequest.new( + kind: ImageGen::Kind::Generate, + prompt: "A plain mid grey background. No text, no logos, no objects.", + aspect: "1:1", + quality: "draft", + candidates: 1, + model: model, + )) + + TestResult.new(ok: true, latency_ms: (Time.utc - started).total_milliseconds.to_i64, model: model) + rescue ex : Error::ImageGen + TestResult.new(false, (Time.utc - started).total_milliseconds.to_i64, model, ex.message, ex.kind) + rescue ex + TestResult.new(false, (Time.utc - started).total_milliseconds.to_i64, model, ex.message, "vendor") + end + end + + private def parse_provider(value : String?) : ::PlaceOS::Model::SignageAIProvider::Provider + return ::PlaceOS::Model::SignageAIProvider::Provider::OPENAI if value.nil? + ::PlaceOS::Model::SignageAIProvider::Provider.parse(value) + rescue ArgumentError + raise Error::ModelValidation.new([Error::Field.new(:provider, "must be one of OPENAI, AZURE_OPENAI, GOOGLE_VERTEX")]) + end + + private def apply_optional(row : ::PlaceOS::Model::SignageAIProvider, params : ProviderParams) : Nil + row.endpoint = params.endpoint if params.endpoint + row.location = params.location if params.location + row.default_model = params.default_model if params.default_model + row.allowed_models = params.allowed_models.not_nil! if params.allowed_models + row.enabled = params.enabled.not_nil! unless params.enabled.nil? + row.is_default = params.is_default.not_nil! unless params.is_default.nil? + + if (quotas = params.quotas) && quotas.as_h? + row.quotas = quotas + end + end + end +end diff --git a/src/placeos-rest-api/error.cr b/src/placeos-rest-api/error.cr index e6abe66d..e8a09906 100644 --- a/src/placeos-rest-api/error.cr +++ b/src/placeos-rest-api/error.cr @@ -29,6 +29,53 @@ module PlaceOS::Api class GuestAccessDisabled < Error end + # Signage AI image generation. Each subclass maps to its own status code in + # `Application`, and renders `{"error": ..., "kind": ...}` so a client can + # branch on the kind without matching on message text. + class ImageGen < Error + # the caller has used up a per user or per domain quota + class Quota < ImageGen + end + + # the vendor refused the prompt or the source image + class Moderated < ImageGen + end + + # no free slot on this replica, the caller should try again shortly + class Busy < ImageGen + end + + # the vendor returned something we could not use + class Vendor < ImageGen + end + + # the vendor did not answer in time + class Timeout < ImageGen + end + + # the caller may not use the referenced upload or item + class Permission < ImageGen + end + + # no provider row, no storage, or the feature is switched off + class NotConfigured < ImageGen + end + + # kind reported to the client, e.g. "moderation", "quota" + def kind : String + case self + when Quota then "quota" + when Moderated then "moderation" + when Busy then "busy" + when Vendor then "vendor" + when Timeout then "timeout" + when Permission then "permission" + when NotConfigured then "not_configured" + else "error" + end + end + end + class ModelValidation < Error getter failures : Array(NamedTuple(field: Symbol, reason: String)) diff --git a/src/placeos-rest-api/utilities/image_gen.cr b/src/placeos-rest-api/utilities/image_gen.cr new file mode 100644 index 00000000..7d60b49c --- /dev/null +++ b/src/placeos-rest-api/utilities/image_gen.cr @@ -0,0 +1,52 @@ +require "./image_gen/types" +require "./image_gen/http" +require "./image_gen/prompt" +require "./image_gen/slots" +require "./image_gen/adapter" +require "./image_gen/adapters/openai_images" +require "./image_gen/adapters/gemini_vertex" +require "./image_gen/store" +require "./image_gen/runner" +require "./image_gen/sweep" + +module PlaceOS::Api + # Image generation for signage artwork. + # + # A request handler validates, reserves slots and writes a job row, then hands + # off to `Runner` in a spawned fiber. Candidates are stored through the same + # `Storage` and `Upload` machinery the uploads controller uses, and written + # back into the job row one at a time so a long polling client sees each one + # as it lands. + # + # Nothing in here touches `request`: the fiber outlives it. + module ImageGen + Log = ::Log.for(self) + + # sizes we ask a vendor for, by aspect. gpt-image-2 needs both edges to be a + # multiple of 16, which is why the landscape size is 2048x1152 and not + # 1920x1080. Players scale to the panel. + SIZES = { + "16:9" => {2048, 1152}, + "9:16" => {1152, 2048}, + "1:1" => {2048, 2048}, + "4:3" => {2048, 1536}, + } + + ASPECTS = SIZES.keys + + QUALITIES = ["standard", "high"] + + MAX_CANDIDATES = 4 + + def self.aspect_valid?(aspect : String) : Bool + SIZES.has_key?(aspect) + end + + def self.size_for(aspect : String) : Tuple(Int32, Int32) + SIZES[aspect]? || SIZES["16:9"] + end + + # shared across the process, sized by SIGNAGE_AI_MAX_CALLS + class_getter slots : Slots { Slots.new(SIGNAGE_AI_MAX_CALLS) } + end +end diff --git a/src/placeos-rest-api/utilities/image_gen/adapter.cr b/src/placeos-rest-api/utilities/image_gen/adapter.cr new file mode 100644 index 00000000..174058e0 --- /dev/null +++ b/src/placeos-rest-api/utilities/image_gen/adapter.cr @@ -0,0 +1,75 @@ +require "placeos-models/signage_ai_provider" + +module PlaceOS::Api::ImageGen + # A vendor. Adapters take an `AdapterRequest` and return images, or raise: + # + # - `Error::ImageGen::Moderated` when the vendor refused the prompt or source + # - `Error::ImageGen::Vendor` for anything else the vendor said + # - `IO::TimeoutError` when it did not answer, mapped to 504 upstream + # + # They never see a job row, a model or the request. + abstract class Adapter + getter row : ::PlaceOS::Model::SignageAIProvider + + def initialize(@row : ::PlaceOS::Model::SignageAIProvider) + end + + abstract def capabilities : ProviderCapabilities + abstract def generate(request : AdapterRequest) : Array(AdapterImage) + abstract def edit(request : AdapterRequest) : Array(AdapterImage) + + # how many images one call can return. OpenAI answers with every candidate + # from a single call, Google returns one image per call, so the runner + # divides the work accordingly and reserves a slot for each call. + abstract def images_per_call : Int32 + + # number of vendor calls needed for this many candidates + def calls_for(candidates : Int32) : Int32 + per_call = images_per_call + return candidates if per_call <= 1 + (candidates + per_call - 1) // per_call + end + + def call(request : AdapterRequest) : Array(AdapterImage) + case request.kind + in Kind::Generate then generate(request) + in Kind::Edit then edit(request) + end + end + + def self.for(row : ::PlaceOS::Model::SignageAIProvider) : Adapter + case row.provider + in ::PlaceOS::Model::SignageAIProvider::Provider::OPENAI, + ::PlaceOS::Model::SignageAIProvider::Provider::AZURE_OPENAI + Adapters::OpenAIImages.new(row) + in ::PlaceOS::Model::SignageAIProvider::Provider::GOOGLE_VERTEX + Adapters::GeminiVertex.new(row) + end + end + + # models a row is allowed to use, its own list or the adapter's defaults + protected def allowed(defaults : Array(ModelCapabilities)) : Array(ModelCapabilities) + allow = row.allowed_models + return defaults if allow.empty? + defaults.select { |model| allow.includes?(model.id) } + end + + protected def credentials : Hash(String, JSON::Any) + row.credentials_json + rescue ex : ::PlaceOS::Model::Error + raise Error::ImageGen::NotConfigured.new(ex.message || "provider credentials could not be read") + end + + protected def credential(key : String) : String + value = credentials[key]?.try(&.as_s?) + raise Error::ImageGen::NotConfigured.new("provider #{row.name} is missing #{key}") if value.nil? || value.empty? + value + end + + # trimmed so a vendor body never lands whole in the logs or an API response + protected def vendor_message(body : String, limit : Int32 = 300) : String + cleaned = body.gsub(/\s+/, " ").strip + cleaned.size > limit ? "#{cleaned[0, limit]}..." : cleaned + end + end +end diff --git a/src/placeos-rest-api/utilities/image_gen/adapters/gemini_vertex.cr b/src/placeos-rest-api/utilities/image_gen/adapters/gemini_vertex.cr new file mode 100644 index 00000000..58d2a54a --- /dev/null +++ b/src/placeos-rest-api/utilities/image_gen/adapters/gemini_vertex.cr @@ -0,0 +1,207 @@ +require "google" +require "json" + +module PlaceOS::Api::ImageGen::Adapters + # Gemini image models on Vertex. + # + # Only the Vertex path is supported: it is the channel that carries Google's + # indemnity and its no training terms, which an AI Studio key does not, so a + # row without a service account is refused rather than quietly downgraded. + # + # Sizes are an enum and a ratio rather than pixels, so an output is close to + # the requested shape but not exact; callers crop. One image comes back per + # call, and at 2K the response can carry more than one image part, of which + # the last is the requested size. + class GeminiVertex < Adapter + DEFAULT_MODEL = "gemini-3.1-flash-image" + DEFAULT_HOST = "https://aiplatform.googleapis.com" + SCOPE = "https://www.googleapis.com/auth/cloud-platform" + + # Google's image models are served from the global endpoint only. Its own + # docs say not to use that endpoint where the processing region matters. + LOCATION = "global" + + MODELS = [ + ModelCapabilities.new( + id: "gemini-3.1-flash-image", + name: "Nano Banana 2", + max_references: 10, + ), + ModelCapabilities.new( + id: "gemini-3-pro-image", + name: "Nano Banana Pro", + max_references: 6, + ), + ] + + # The shard caches tokens in a process wide hash keyed on scope and subject + # only, so two service accounts with the same scope would share an entry. + # Key on the issuer as well. + class Auth < Google::ServiceAuth + private def token_lookup + "#{@issuer}_#{super}" + end + end + + def capabilities : ProviderCapabilities + ProviderCapabilities.new( + id: row.id.to_s, + name: row.name, + provider: row.provider.to_s, + models: allowed(MODELS), + default_model: row.default_model || DEFAULT_MODEL, + region: LOCATION, + ) + end + + def images_per_call : Int32 + 1 + end + + def generate(request : AdapterRequest) : Array(AdapterImage) + call(build_parts(request), request) + end + + def edit(request : AdapterRequest) : Array(AdapterImage) + source = request.source + raise Error::ImageGen::Vendor.new("an edit needs a source image") if source.nil? + call(build_parts(request, source), request) + end + + private def build_parts(request : AdapterRequest, source : Reference? = nil) : Array(JSON::Any) + parts = [] of JSON::Any + + if source + parts << inline(source) + end + request.references.each { |reference| parts << inline(reference) } + parts << JSON::Any.new({"text" => JSON::Any.new(request.prompt)}) + + parts + end + + private def inline(reference : Reference) : JSON::Any + JSON::Any.new({ + "inlineData" => JSON::Any.new({ + "mimeType" => JSON::Any.new(reference.mime), + "data" => JSON::Any.new(Base64.strict_encode(reference.bytes)), + }), + }) + end + + private def call(parts : Array(JSON::Any), request : AdapterRequest) : Array(AdapterImage) + body = { + contents: [ + {role: "user", parts: parts}, + ], + generationConfig: { + responseModalities: ["IMAGE"], + imageConfig: { + aspectRatio: request.aspect, + imageSize: image_size(request.quality), + outputMimeType: "image/jpeg", + outputCompressionQuality: 90, + }, + }, + }.to_json + + uri = url_for(request.model) + response = Http.client(uri) do |client| + client.post(uri.request_target, headers: headers, body: body) + end + + raise_for_status(response) + parse(response.body) + end + + # 1K is a draft, everything else is 2K. 4K is not offered: the two open + # regressions Google has on 2K and 4K image to image sit on this path. + private def image_size(quality : String) : String + quality == "draft" ? "1K" : "2K" + end + + private def project : String + credentials["project_id"]?.try(&.as_s?) || credential("project") + end + + private def url_for(model : String) : URI + host = row.endpoint.presence.try(&.rstrip('/')) || DEFAULT_HOST + URI.parse("#{host}/v1/projects/#{project}/locations/#{LOCATION}/publishers/google/models/#{model}:generateContent") + end + + private def headers : HTTP::Headers + HTTP::Headers{ + "Content-Type" => "application/json", + "Authorization" => "Bearer #{token}", + } + end + + private def token : String + issuer = credential("client_email") + key = credential("private_key") + Auth.new(issuer: issuer, signing_key: key, scopes: SCOPE).get_token.access_token + rescue ex : Error::ImageGen + raise ex + rescue ex + raise Error::ImageGen::NotConfigured.new("could not authenticate with Google: #{ex.message}") + end + + private def raise_for_status(response : HTTP::Client::Response) : Nil + return if response.success? + + message = nil + status = nil + begin + error = JSON.parse(response.body)["error"]? + message = error.try(&.["message"]?).try(&.as_s?) + status = error.try(&.["status"]?).try(&.as_s?) + rescue JSON::ParseException + end + + raise Error::ImageGen::Busy.new(message || "Google is rate limiting this project") if response.status_code == 429 + raise Error::ImageGen::Moderated.new(message || "the request was blocked") if status == "PERMISSION_DENIED" && message.try(&.includes?("safety")) + raise Error::ImageGen::Vendor.new(message || vendor_message(response.body)) + end + + private def parse(body : String) : Array(AdapterImage) + payload = JSON.parse(body) + + if (reason = payload["promptFeedback"]?.try(&.["blockReason"]?).try(&.as_s?)) + raise Error::ImageGen::Moderated.new("the prompt was blocked (#{reason})") + end + + candidate = payload["candidates"]?.try(&.as_a?).try(&.first?) + raise Error::ImageGen::Vendor.new("no candidates in the vendor response") if candidate.nil? + + if (finish = candidate["finishReason"]?.try(&.as_s?)) + case finish + when "IMAGE_SAFETY", "PROHIBITED_CONTENT", "SAFETY" + raise Error::ImageGen::Moderated.new("the image was blocked by Google's safety system (#{finish})") + end + end + + parts = candidate["content"]?.try(&.["parts"]?).try(&.as_a?) || [] of JSON::Any + + # at 2K the response can carry more than one image part; the last is the + # one at the requested size + inline = parts.compact_map { |part| part["inlineData"]? || part["inline_data"]? }.last? + raise Error::ImageGen::Vendor.new("no image in the vendor response") if inline.nil? + + encoded = inline["data"]?.try(&.as_s?) + raise Error::ImageGen::Vendor.new("no image data in the vendor response") if encoded.nil? + + bytes = Base64.decode(encoded) + dimensions = Http.dimensions(bytes) + cost = payload["usageMetadata"]?.try(&.["totalTokenCount"]?).try(&.as_i64?).try(&.to_f) + + [AdapterImage.new( + bytes: bytes, + mime: inline["mimeType"]?.try(&.as_s?) || "image/jpeg", + width: dimensions.try(&.[0]), + height: dimensions.try(&.[1]), + vendor_id: payload["responseId"]?.try(&.as_s?), + cost_units: cost, + )] + end + end +end diff --git a/src/placeos-rest-api/utilities/image_gen/adapters/openai_images.cr b/src/placeos-rest-api/utilities/image_gen/adapters/openai_images.cr new file mode 100644 index 00000000..9881861a --- /dev/null +++ b/src/placeos-rest-api/utilities/image_gen/adapters/openai_images.cr @@ -0,0 +1,193 @@ +require "http/formdata" +require "json" + +module PlaceOS::Api::ImageGen::Adapters + # OpenAI's image API, and Azure OpenAI, which speaks the same request and + # response shape at a different base URL with a different auth header. + # + # gpt-image-2 wants both edges to be a multiple of 16, returns base64 in + # `data[].b64_json`, and can return every candidate from one call, so a + # request of n candidates costs one vendor call. + class OpenAIImages < Adapter + DEFAULT_MODEL = "gpt-image-2" + DEFAULT_BASE = "https://api.openai.com/v1" + DEFAULT_VERSION = "2026-04-01-preview" + + MODELS = [ + ModelCapabilities.new( + id: "gpt-image-2", + name: "GPT Image 2", + max_references: 16, + ), + ] + + def capabilities : ProviderCapabilities + ProviderCapabilities.new( + id: row.id.to_s, + name: row.name, + provider: row.provider.to_s, + models: allowed(MODELS), + default_model: row.default_model || DEFAULT_MODEL, + region: row.location, + ) + end + + def images_per_call : Int32 + MAX_CANDIDATES + end + + def generate(request : AdapterRequest) : Array(AdapterImage) + body = { + model: request.model, + prompt: request.prompt, + n: request.candidates, + size: request.size, + quality: quality(request.quality), + output_format: "jpeg", + moderation: "auto", + }.to_json + + post("images/generations", body, "application/json") + end + + def edit(request : AdapterRequest) : Array(AdapterImage) + source = request.source + raise Error::ImageGen::Vendor.new("an edit needs a source image") if source.nil? + + io = IO::Memory.new + content_type = "" + HTTP::FormData.build(io) do |form| + content_type = form.content_type + form.field("model", request.model) + form.field("prompt", request.prompt) + form.field("n", request.candidates.to_s) + form.field("size", request.size) + form.field("quality", quality(request.quality)) + form.field("output_format", "jpeg") + + # the image being edited goes first, references after it + add_image(form, source, "source") + request.references.each_with_index do |reference, index| + add_image(form, reference, "reference-#{index}") + end + end + + post("images/edits", io.to_s, content_type) + end + + private def add_image(form : HTTP::FormData::Builder, reference : Reference, name : String) : Nil + metadata = HTTP::FormData::FileMetadata.new(filename: "#{name}.#{extension(reference.mime)}") + headers = HTTP::Headers{"Content-Type" => reference.mime} + form.file("image[]", IO::Memory.new(reference.bytes), metadata, headers) + end + + private def extension(mime : String) : String + case mime + when "image/png" then "png" + when "image/webp" then "webp" + else "jpg" + end + end + + # "standard" is the vendor's medium tier, which is what the crowd + # leaderboards rank first and what iteration runs at. "high" is the + # explicit enhance step. + private def quality(value : String) : String + value == "high" ? "high" : "medium" + end + + private def azure? : Bool + row.provider.azure_openai? + end + + private def base : String + if (endpoint = row.endpoint.presence) + endpoint.rstrip('/') + elsif azure? + raise Error::ImageGen::NotConfigured.new("Azure OpenAI provider #{row.name} needs an endpoint") + else + DEFAULT_BASE + end + end + + private def url_for(path : String) : URI + if azure? + deployment = credential("deployment") + version = credentials["api_version"]?.try(&.as_s?) || DEFAULT_VERSION + URI.parse("#{base}/openai/deployments/#{deployment}/#{path}?api-version=#{version}") + else + URI.parse("#{base}/#{path}") + end + end + + private def headers(content_type : String) : HTTP::Headers + headers = HTTP::Headers{"Content-Type" => content_type} + if azure? + headers["api-key"] = credential("api_key") + else + headers["Authorization"] = "Bearer #{credential("api_key")}" + if organisation = credentials["organisation"]?.try(&.as_s?).presence + headers["OpenAI-Organization"] = organisation + end + end + headers + end + + private def post(path : String, body : String, content_type : String) : Array(AdapterImage) + uri = url_for(path) + response = Http.client(uri) do |client| + client.post(uri.request_target, headers: headers(content_type), body: body) + end + + raise_for_status(response) + parse(response.body) + end + + private def raise_for_status(response : HTTP::Client::Response) : Nil + return if response.success? + + code = nil + message = nil + begin + error = JSON.parse(response.body)["error"]? + code = error.try(&.["code"]?).try(&.as_s?) + message = error.try(&.["message"]?).try(&.as_s?) + rescue JSON::ParseException + end + + if code == "moderation_blocked" || code == "content_policy_violation" + raise Error::ImageGen::Moderated.new(message || "the request was blocked by the vendor's safety system") + end + + if response.status_code == 429 + raise Error::ImageGen::Busy.new(message || "the vendor is rate limiting this account") + end + + raise Error::ImageGen::Vendor.new(message || vendor_message(response.body)) + end + + private def parse(body : String) : Array(AdapterImage) + payload = JSON.parse(body) + + cost = payload["usage"]?.try(&.["total_tokens"]?).try(&.as_i64?).try(&.to_f) + images = payload["data"]?.try(&.as_a?) + raise Error::ImageGen::Vendor.new("no images in the vendor response") if images.nil? || images.empty? + + per_image = cost ? cost / images.size : nil + + images.compact_map do |entry| + encoded = entry["b64_json"]?.try(&.as_s?) + next nil unless encoded + bytes = Base64.decode(encoded) + dimensions = Http.dimensions(bytes) + AdapterImage.new( + bytes: bytes, + mime: "image/jpeg", + width: dimensions.try(&.[0]), + height: dimensions.try(&.[1]), + cost_units: per_image, + ) + end + end + end +end diff --git a/src/placeos-rest-api/utilities/image_gen/http.cr b/src/placeos-rest-api/utilities/image_gen/http.cr new file mode 100644 index 00000000..b3383a91 --- /dev/null +++ b/src/placeos-rest-api/utilities/image_gen/http.cr @@ -0,0 +1,85 @@ +require "connect-proxy" + +module PlaceOS::Api::ImageGen + # One place that builds an HTTP client for a vendor call. + # + # `ConnectProxy::HTTPClient` honours the proxy environment the helm chart sets, + # which a plain `HTTP::Client` ignores. Timeouts are always explicit: no other + # rest-api code sets a read timeout, and a vendor that stops responding would + # otherwise hold a fiber and its slot forever. + module Http + CONNECT_TIMEOUT = 10.seconds + + def self.client(uri : URI, read_timeout : Time::Span = SIGNAGE_AI_READ_TIMEOUT, & : HTTP::Client -> _) + client = ConnectProxy::HTTPClient.new(uri) + client.connect_timeout = CONNECT_TIMEOUT + client.read_timeout = read_timeout + client.write_timeout = read_timeout + begin + yield client + ensure + client.close rescue nil + end + end + + # Fetch bytes we already hold a signed URL for. Crystal's HTTP::Client does + # not follow redirects, so this follows a small number by hand: `/uploads/:id/url` + # answers with a 303 to the storage provider. + def self.get_bytes(url : String, limit : Int32 = 3) : Tuple(Bytes, String) + current = url + limit.times do + uri = URI.parse(current) + response = client(uri, 60.seconds) { |http| http.get(uri.request_target) } + + if response.status.redirection? && (location = response.headers["Location"]?) + current = location.starts_with?("http") ? location : URI.parse(current).resolve(location).to_s + next + end + + raise Error::ImageGen::Vendor.new("could not read image (#{response.status_code})") unless response.success? + mime = response.headers["Content-Type"]? || "application/octet-stream" + return {response.body.to_slice, mime.split(';').first.strip} + end + raise Error::ImageGen::Vendor.new("too many redirects reading image") + end + + # Width and height straight out of the file header. The api binary is built + # `--static` from scratch and has no image library, and we only ever deal + # with JPEG and PNG here. + def self.dimensions(bytes : Bytes) : Tuple(Int32, Int32)? + return png_dimensions(bytes) if bytes.size > 24 && bytes[0] == 0x89 && bytes[1] == 0x50 + return jpeg_dimensions(bytes) if bytes.size > 4 && bytes[0] == 0xFF && bytes[1] == 0xD8 + nil + end + + private def self.png_dimensions(bytes : Bytes) : Tuple(Int32, Int32)? + # IHDR is always the first chunk: width and height are big endian at 16..23 + width = IO::ByteFormat::BigEndian.decode(UInt32, bytes[16, 4]) + height = IO::ByteFormat::BigEndian.decode(UInt32, bytes[20, 4]) + {width.to_i32, height.to_i32} + rescue + nil + end + + private def self.jpeg_dimensions(bytes : Bytes) : Tuple(Int32, Int32)? + index = 2 + while index + 9 < bytes.size + return nil unless bytes[index] == 0xFF + marker = bytes[index + 1] + length = IO::ByteFormat::BigEndian.decode(UInt16, bytes[index + 2, 2]).to_i + + # SOF0..SOF15, skipping the four that are not frame headers + if marker >= 0xC0 && marker <= 0xCF && marker != 0xC4 && marker != 0xC8 && marker != 0xCC + height = IO::ByteFormat::BigEndian.decode(UInt16, bytes[index + 5, 2]).to_i32 + width = IO::ByteFormat::BigEndian.decode(UInt16, bytes[index + 7, 2]).to_i32 + return {width, height} + end + + index += 2 + length + end + nil + rescue + nil + end + end +end diff --git a/src/placeos-rest-api/utilities/image_gen/prompt.cr b/src/placeos-rest-api/utilities/image_gen/prompt.cr new file mode 100644 index 00000000..f438b89a --- /dev/null +++ b/src/placeos-rest-api/utilities/image_gen/prompt.cr @@ -0,0 +1,117 @@ +require "json" + +module PlaceOS::Api::ImageGen + # Turns a brief, the domain's brand kit and a refine chain into the text a + # vendor is sent. + # + # The words on a finished poster are drawn by the browser on a text layer, not + # by the model, so unless the caller opts out the prompt asks for a clear area + # and for no lettering at all. The logo is composited from the customer's own + # file for the same reason, so the prompt asks for room rather than a drawing + # of it. + module Prompt + # `signage_ai` metadata on the org zone + struct BrandKit + include JSON::Serializable + + getter organisation : String? = nil + getter palette : Hash(String, String)? = nil + getter tone : String? = nil + getter logo_upload_id : String? = nil + getter never_include : Array(String) = [] of String + + @[JSON::Field(ignore: true)] + getter font : JSON::Any? = nil + + def initialize + end + + def self.parse(value : JSON::Any?) : BrandKit? + return nil unless value + BrandKit.from_json(value.to_json) + rescue JSON::SerializableError | JSON::ParseException + nil + end + + def palette_line : String? + colours = palette + return nil if colours.nil? || colours.empty? + colours.map { |name, hex| "#{name} #{hex}" }.join(", ") + end + end + + # Where the browser will put things, so the model leaves room. + enum TextMode + # the app draws the words (default) + Layer + # the caller asked the model to render the words itself + Model + end + + record Options, + brief : String, + aspect : String, + text_mode : TextMode = TextMode::Layer, + include_logo : Bool = true, + brand : BrandKit? = nil, + words : String? = nil, + history : Array(String) = [] of String, + instruction : String? = nil + + def self.build(options : Options) : String + lines = [] of String + + if (brand = options.brand) + parts = [] of String + parts << "Organisation: #{brand.organisation}." if brand.organisation.presence + if (colours = brand.palette_line) + parts << "Palette: #{colours}." + end + parts << "Tone: #{brand.tone}." if brand.tone.presence + lines << "Brand: #{parts.join(" ")}" unless parts.empty? + end + + lines << "Layout: #{layout_line(options)}" + lines << "Brief: #{options.brief}" if options.brief.presence + + options.history.each_with_index do |entry, index| + lines << "Change #{index + 1}: #{entry}" + end + + if (instruction = options.instruction) && instruction.presence + lines << "Change #{options.history.size + 1}: #{instruction}" + lines << "Keep everything else exactly as it is." + end + + if (brand = options.brand) && !brand.never_include.empty? + lines << "Never include: #{brand.never_include.join(", ")}." + end + + lines.join("\n") + end + + private def self.layout_line(options : Options) : String + orientation = case options.aspect + when "9:16" then "Portrait" + when "1:1" then "Square" + else "Landscape" + end + + parts = ["#{orientation} #{options.aspect} poster background for a digital signage screen."] + + case options.text_mode + in TextMode::Layer + parts << "Leave the top third clear and plain so a headline can be added over it." + parts << "Leave a clear area in the bottom right corner for a logo." if options.include_logo + parts << "Do not render any text, letters, numbers, words or logos anywhere in the image." + in TextMode::Model + if (words = options.words) && words.presence + parts << "Render exactly this text and nothing else: #{words.inspect}." + end + parts << "Leave a clear area in the bottom right corner for a logo." if options.include_logo + end + + parts.join(" ") + end + end +end diff --git a/src/placeos-rest-api/utilities/image_gen/runner.cr b/src/placeos-rest-api/utilities/image_gen/runner.cr new file mode 100644 index 00000000..f85371ce --- /dev/null +++ b/src/placeos-rest-api/utilities/image_gen/runner.cr @@ -0,0 +1,206 @@ +require "placeos-models/signage_ai_job" +require "placeos-models/storage" +require "placeos-models/user" + +module PlaceOS::Api::ImageGen + # The body of the spawned fiber. + # + # Everything it needs is captured before `spawn`: inside the fiber there is no + # request, `Log.context` is empty and `current_user` does not exist. Candidate + # results are written back one at a time with an atomic update so a long + # polling client sees each one land, and so two candidate fibers cannot drop + # each other's entry. + module Runner + # Captured at request time and handed to the fiber. + record Context, + job_id : UUID, + authority_id : String, + hostname : String, + user : ::PlaceOS::Model::User, + storage : ::PlaceOS::Model::Storage, + adapter : Adapter, + request : AdapterRequest, + source_upload_id : String? = nil, + reference_upload_ids : Array(String) = [] of String + + def self.run(context : Context) : Nil + ::Log.context.set( + signage_ai_job: context.job_id.to_s, + authority_id: context.authority_id, + user_id: context.user.id, + ) + + started = Time.utc + job = ::PlaceOS::Model::SignageAIJob.find?(context.job_id) + return if job.nil? + + job.state = ::PlaceOS::Model::SignageAIJob::State::Running + job.started_at = started + job.version = job.version + 1 + job.save + + request = hydrate(context) + + produced = 0 + failure : Exception? = nil + cost = 0.0 + + calls = plan(context.adapter, context.request.candidates) + index_offset = 0 + done = Channel(Nil).new + + calls.each_with_index do |wanted, call_index| + offset = index_offset + index_offset += wanted + + spawn do + begin + next if cancelled?(context.job_id) + + images = context.adapter.call(request.copy_with(candidates: wanted)) + + images.each_with_index do |image, image_index| + slot = offset + image_index + next if slot >= context.request.candidates + + stored = Store.put(image, context.storage, context.user, context.hostname, context.job_id.to_s, slot) + cost += image.cost_units || 0.0 + produced += 1 + + ::PlaceOS::Model::SignageAIJob.bump_image(context.job_id, slot, { + "state" => JSON::Any.new("done"), + "index" => JSON::Any.new(slot.to_i64), + "upload_id" => JSON::Any.new(stored.upload.id.as(String)), + "url" => JSON::Any.new("/api/engine/v2/uploads/#{stored.upload.id}/url"), + "width" => stored.width ? JSON::Any.new(stored.width.not_nil!.to_i64) : JSON::Any.new(nil), + "height" => stored.height ? JSON::Any.new(stored.height.not_nil!.to_i64) : JSON::Any.new(nil), + "mime" => JSON::Any.new(image.mime), + "bytes" => JSON::Any.new(image.bytes.size.to_i64), + }) + end + rescue ex + Log.warn(exception: ex) { {message: "signage AI vendor call failed", call: call_index} } + failure ||= ex + ensure + # one release per finished vendor call + ImageGen.slots.release + done.send(nil) + end + end + end + + calls.size.times { done.receive } + + finish(context, produced, cost, failure, started) + rescue ex + Log.error(exception: ex) { "signage AI job failed outside a vendor call" } + fail_job(context.job_id, ex) + # nothing reserved past this point, but a partial plan may still hold slots + end + + # Fetch the source and reference bytes once, before any vendor call. + private def self.hydrate(context : Context) : AdapterRequest + request = context.request + + if (source_id = context.source_upload_id) + upload = ::PlaceOS::Model::Upload.find?(source_id) + raise Error::ImageGen::Vendor.new("the source image is gone") if upload.nil? + request = request.copy_with(source: Store.fetch(upload)) + end + + unless context.reference_upload_ids.empty? + references = context.reference_upload_ids.compact_map do |id| + upload = ::PlaceOS::Model::Upload.find?(id) + upload ? Store.fetch(upload) : nil + end + request = request.copy_with(references: references) + end + + request + end + + # How many images to ask for per vendor call. + private def self.plan(adapter : Adapter, candidates : Int32) : Array(Int32) + per_call = adapter.images_per_call + per_call = 1 if per_call < 1 + + remaining = candidates + calls = [] of Int32 + while remaining > 0 + take = Math.min(per_call, remaining) + calls << take + remaining -= take + end + calls + end + + private def self.cancelled?(job_id : UUID) : Bool + job = ::PlaceOS::Model::SignageAIJob.find?(job_id) + job.nil? || job.cancel_requested + end + + private def self.finish(context : Context, produced : Int32, cost : Float64, failure : Exception?, started : Time) : Nil + job = ::PlaceOS::Model::SignageAIJob.find?(context.job_id) + return if job.nil? + + job.finished_at = Time.utc + job.latency_ms = (Time.utc - started).total_milliseconds.to_i64 + job.cost_units = cost if cost > 0 + job.version = job.version + 1 + + if job.cancel_requested + job.state = ::PlaceOS::Model::SignageAIJob::State::Cancelled + elsif produced > 0 + # a partial result is still a result: the user picks from what landed + job.state = ::PlaceOS::Model::SignageAIJob::State::Done + if failure + job.error_kind = kind_of(failure) + job.error_message = message_of(failure) + end + elsif failure + job.state = ::PlaceOS::Model::SignageAIJob::State::Failed + job.error_kind = kind_of(failure) + job.error_message = message_of(failure) + else + job.state = ::PlaceOS::Model::SignageAIJob::State::Failed + job.error_kind = "vendor" + job.error_message = "no images were produced" + end + + job.save + + Log.info { { + message: "signage AI job finished", + state: job.state.to_s, + produced: produced, + calls: context.request.candidates, + latency: job.latency_ms, + model: context.request.model, + } } + end + + private def self.fail_job(job_id : UUID, error : Exception) : Nil + job = ::PlaceOS::Model::SignageAIJob.find?(job_id) + return if job.nil? + job.state = ::PlaceOS::Model::SignageAIJob::State::Failed + job.error_kind = kind_of(error) + job.error_message = message_of(error) + job.finished_at = Time.utc + job.version = job.version + 1 + job.save + end + + private def self.kind_of(error : Exception) : String + case error + when Error::ImageGen then error.as(Error::ImageGen).kind + when IO::TimeoutError then "timeout" + else "vendor" + end + end + + # never the vendor's whole body, and never the prompt + private def self.message_of(error : Exception) : String + (error.message || error.class.name).lines.first?.to_s[0, 300] + end + end +end diff --git a/src/placeos-rest-api/utilities/image_gen/slots.cr b/src/placeos-rest-api/utilities/image_gen/slots.cr new file mode 100644 index 00000000..275d57dc --- /dev/null +++ b/src/placeos-rest-api/utilities/image_gen/slots.cr @@ -0,0 +1,39 @@ +module PlaceOS::Api::ImageGen + # A per replica cap on concurrent vendor calls. + # + # A request reserves one slot per candidate before its job row is written, all + # or nothing and without blocking. If the reservation fails the caller is told + # the service is busy (503) and no row is created, so there is never a queued + # job nobody is working on. Slots are released one per finished vendor call. + class Slots + getter capacity : Int32 + + def initialize(@capacity : Int32) + @mutex = Mutex.new + @in_use = 0 + end + + # Reserve `count` slots, or none at all. Never blocks. + def try_reserve(count : Int32) : Bool + return false if count <= 0 || count > @capacity + + @mutex.synchronize do + return false if @in_use + count > @capacity + @in_use += count + true + end + end + + # Release one slot. Never drops below zero, so an extra call is harmless. + def release(count : Int32 = 1) : Nil + @mutex.synchronize do + @in_use -= count + @in_use = 0 if @in_use < 0 + end + end + + def available : Int32 + @mutex.synchronize { @capacity - @in_use } + end + end +end diff --git a/src/placeos-rest-api/utilities/image_gen/store.cr b/src/placeos-rest-api/utilities/image_gen/store.cr new file mode 100644 index 00000000..124637ff --- /dev/null +++ b/src/placeos-rest-api/utilities/image_gen/store.cr @@ -0,0 +1,113 @@ +require "digest/md5" +require "upload-signer" +require "placeos-models/storage" +require "placeos-models/upload" + +module PlaceOS::Api::ImageGen + # Writes a generated image into the domain's object storage through the same + # `Storage` and `Upload` machinery the uploads controller uses, from outside a + # request: signs a PUT, sends the bytes with the signature headers verbatim, + # then marks the row complete. + module Store + CANDIDATE_TAG = "ai-candidate" + REFERENCE_TAG = "ai-reference" + + record Stored, upload : ::PlaceOS::Model::Upload, width : Int32?, height : Int32? + + def self.signer_for(storage : ::PlaceOS::Model::Storage) : UploadSigner::Storage + UploadSigner.signer( + UploadSigner::StorageType.from_value(storage.storage_type.value), + storage.access_key, + storage.decrypt_secret, + storage.region, + endpoint: storage.endpoint, + ) + end + + # `hostname` comes from the request that started the job: the fiber has no + # request, and the object key convention starts with the domain. + def self.put( + image : AdapterImage, + storage : ::PlaceOS::Model::Storage, + user : ::PlaceOS::Model::User, + hostname : String, + job_id : String, + index : Int32, + ) : Stored + extension = case image.mime + when "image/png" then "png" + when "image/webp" then "webp" + else "jpg" + end + + file_name = "ai-#{job_id}-#{index}.#{extension}" + object_key = "/#{hostname}/ai/#{job_id}/#{index}.#{extension}" + md5 = Digest::MD5.base64digest(image.bytes) + + upload = ::PlaceOS::Model::Upload.new( + uploaded_by: user.id.as(String), + uploaded_email: user.email, + file_name: file_name, + file_size: image.bytes.size.to_i64, + file_md5: md5, + storage_id: storage.id, + object_key: object_key, + # candidates are private until the user keeps one + public: false, + permissions: ::PlaceOS::Model::Upload::Permissions::None, + object_options: { + "permissions" => JSON::Any.new("private"), + "headers" => JSON::Any.new({"Content-Type" => JSON::Any.new(image.mime)}), + }, + tags: [CANDIDATE_TAG, "ai-job-#{job_id}"], + ) + raise Error::ImageGen::Vendor.new("could not record the generated image") unless upload.save + + signer = signer_for(storage) + signature = signer.sign_upload( + storage.bucket_name, + object_key, + image.bytes.size.to_i64, + md5, + image.mime, + :private, + 5.minutes.total_seconds.to_i, + {"Content-Type" => image.mime}, + ) + + uri = URI.parse(signature[:url]) + headers = HTTP::Headers.new + # the signature covers these exactly as given, so they go on the wire + # unchanged + signature[:headers].each { |key, value| headers[key] = value } + + response = Http.client(uri, 120.seconds) do |client| + client.exec(signature[:verb].upcase, uri.request_target, headers: headers, body: image.bytes) + end + + unless response.success? + upload.destroy rescue nil + raise Error::ImageGen::Vendor.new("storage rejected the image (#{response.status_code})") + end + + upload.update!(upload_complete: true) + + Stored.new(upload: upload, width: image.width, height: image.height) + end + + # Read an upload back out, for a source image or a reference. + def self.fetch(upload : ::PlaceOS::Model::Upload) : Reference + storage = upload.storage + raise Error::ImageGen::NotConfigured.new("upload #{upload.id} has no storage") if storage.nil? + + url = signer_for(storage).get_object(storage.bucket_name, upload.object_key, 5.minutes.total_seconds.to_i) + bytes, mime = Http.get_bytes(url) + + # trust the file header over whatever the bucket reported + mime = "image/png" if bytes.size > 8 && bytes[0] == 0x89 && bytes[1] == 0x50 + mime = "image/jpeg" if bytes.size > 3 && bytes[0] == 0xFF && bytes[1] == 0xD8 + + Reference.new(bytes: bytes, mime: mime) + end + end +end diff --git a/src/placeos-rest-api/utilities/image_gen/sweep.cr b/src/placeos-rest-api/utilities/image_gen/sweep.cr new file mode 100644 index 00000000..f0628a4c --- /dev/null +++ b/src/placeos-rest-api/utilities/image_gen/sweep.cr @@ -0,0 +1,106 @@ +require "placeos-driver/storage" +require "placeos-models/playlist/item" +require "placeos-models/signage_ai_job" +require "placeos-models/upload" + +module PlaceOS::Api::ImageGen + # Housekeeping. + # + # Candidates nobody kept, and reference images uploaded for one request, are + # removed after the retention window. A candidate that was kept is left alone: + # it is the media item's file, and its tag is the provenance record. Jobs left + # running by a replica that went away are marked failed so a client stops + # waiting on them. + # + # One replica does the work at a time, held by a Redis key. Nothing slow + # happens inside the `with_redis` block: it holds a single shared client + # behind a mutex. + module Sweep + LOCK_KEY = "placeos/signage_ai/sweep" + + def self.start : Tasker::Repeat(Nil)? + return nil if SIGNAGE_AI_DISABLED + + period = SIGNAGE_AI_RETENTION / 8 + period = 15.minutes if period < 15.minutes + + Log.info { {message: "signage AI sweep scheduled", every: period.to_s, retention: SIGNAGE_AI_RETENTION.to_s} } + + Tasker.instance.every(period) do + begin + run + rescue ex + Log.error(exception: ex) { "signage AI sweep failed" } + end + nil + end + end + + def self.run : Nil + ttl = 10.minutes.total_seconds.to_i + # SET NX EX in one call: taken and released outside any long work + taken = ::PlaceOS::Driver::RedisStorage.with_redis do |redis| + redis.set(LOCK_KEY, Time.utc.to_unix.to_s, nx: true, ex: ttl) + end + return unless taken + + begin + expire_stale_jobs + remove_unclaimed_uploads + ensure + ::PlaceOS::Driver::RedisStorage.with_redis(&.del(LOCK_KEY)) + end + end + + # a job still running long after the replica that owned it went away + private def self.expire_stale_jobs : Nil + cutoff = Time.utc - SIGNAGE_AI_JOB_STALE + ::PlaceOS::Model::SignageAIJob.stale(cutoff).each do |job| + job.state = ::PlaceOS::Model::SignageAIJob::State::Failed + job.error_kind = "timeout" + job.error_message = "the job did not finish" + job.finished_at = Time.utc + job.version = job.version + 1 + job.save + Log.warn { {message: "signage AI job expired", job: job.id.to_s} } + end + end + + private def self.remove_unclaimed_uploads : Nil + cutoff = Time.utc - SIGNAGE_AI_RETENTION + + # bind each tag individually: an array cannot go in as one parameter + tags = [Store::CANDIDATE_TAG, Store::REFERENCE_TAG] + placeholders = Array.new(tags.size, "?").join(", ") + args = ([cutoff] of ::PgORM::Value) + tags.map(&.as(::PgORM::Value)) + + uploads = ::PlaceOS::Model::Upload + .where("created_at < ? AND tags && ARRAY[#{placeholders}]::text[]", args: args) + .to_a + + return if uploads.empty? + + uploads.each do |upload| + id = upload.id.as(String) + next if referenced?(id) + + begin + if (storage = upload.storage) + Store.signer_for(storage).delete_file(storage.bucket_name, upload.object_key, upload.resumable_id) + end + rescue ex + Log.warn(exception: ex) { {message: "could not remove a swept object", upload: id} } + end + + upload.destroy + end + end + + # kept if a media item points at it, either as the artwork or its thumbnail + private def self.referenced?(upload_id : String) : Bool + ::PlaceOS::Model::Playlist::Item + .where("media_id = ? OR thumbnail_id = ?", upload_id, upload_id) + .count > 0 + end + end +end diff --git a/src/placeos-rest-api/utilities/image_gen/types.cr b/src/placeos-rest-api/utilities/image_gen/types.cr new file mode 100644 index 00000000..102d9aba --- /dev/null +++ b/src/placeos-rest-api/utilities/image_gen/types.cr @@ -0,0 +1,79 @@ +require "json" + +module PlaceOS::Api::ImageGen + enum Kind + Generate + Edit + end + + # An image handed to a vendor as context. `role` is only used to build the + # prompt text, the vendors take an ordered list. + record Reference, bytes : Bytes, mime : String, role : String = "reference" + + # What an adapter is asked to do. Adapters never see a model, a job row or a + # request: everything they need is here. + record AdapterRequest, + kind : Kind, + prompt : String, + aspect : String, + quality : String, + candidates : Int32, + model : String, + references : Array(Reference) = [] of Reference, + source : Reference? = nil, + options : Hash(String, JSON::Any) = {} of String => JSON::Any do + def width : Int32 + ImageGen.size_for(aspect)[0] + end + + def height : Int32 + ImageGen.size_for(aspect)[1] + end + + def size : String + "#{width}x#{height}" + end + end + + # One image back from a vendor. + record AdapterImage, + bytes : Bytes, + mime : String = "image/jpeg", + width : Int32? = nil, + height : Int32? = nil, + vendor_id : String? = nil, + cost_units : Float64? = nil + + struct ModelCapabilities + include JSON::Serializable + + getter id : String + getter name : String + getter generate : Bool + getter edit : Bool + getter enhance : Bool + getter max_references : Int32 + getter max_candidates : Int32 + getter qualities : Array(String) + getter aspect_ratios : Array(String) + + def initialize(@id, @name, @generate = true, @edit = true, @enhance = true, + @max_references = 8, @max_candidates = MAX_CANDIDATES, + @qualities = QUALITIES, @aspect_ratios = ASPECTS) + end + end + + struct ProviderCapabilities + include JSON::Serializable + + getter id : String + getter name : String + getter provider : String + getter region : String? + getter default_model : String? + getter models : Array(ModelCapabilities) + + def initialize(@id, @name, @provider, @models, @default_model = nil, @region = nil) + end + end +end From 8f48fd1cad758a6bc57a30fa0904ceb069f4eb67 Mon Sep 17 00:00:00 2001 From: Cameron Reeves Date: Fri, 28 Aug 2026 00:18:53 +1000 Subject: [PATCH 02/14] test(signage-ai): controller specs, and square the slot ledger 28 specs across the two controllers: capabilities on and off, the three group permission refusals, quota, the idempotency replay, a full run with the vendor and the storage PUT mocked through to the Upload row, a vendor refusal landing on the job as a moderation failure, both long poll behaviours, cancel, claim, usage, and that credentials never appear in a provider response. Also fixes a slot leak the specs turned up: the runner returned without handing back its reserved slots when the job row had gone, so a replica lost capacity until it restarted. Slots are now tracked in an atomic ledger that every exit path drains exactly once. Co-Authored-By: Claude Opus 5 (1M context) --- .../signage/signage_ai_providers_spec.cr | 150 ++++++ spec/controllers/signage/signage_ai_spec.cr | 482 ++++++++++++++++++ spec/helper.cr | 5 + spec/spec_helpers/http_mocks.cr | 61 +++ .../image_gen/adapters/gemini_vertex.cr | 2 +- .../image_gen/adapters/openai_images.cr | 2 +- .../utilities/image_gen/http.cr | 10 + .../utilities/image_gen/runner.cr | 23 +- .../utilities/image_gen/store.cr | 5 +- 9 files changed, 732 insertions(+), 8 deletions(-) create mode 100644 spec/controllers/signage/signage_ai_providers_spec.cr create mode 100644 spec/controllers/signage/signage_ai_spec.cr diff --git a/spec/controllers/signage/signage_ai_providers_spec.cr b/spec/controllers/signage/signage_ai_providers_spec.cr new file mode 100644 index 00000000..474f5591 --- /dev/null +++ b/spec/controllers/signage/signage_ai_providers_spec.cr @@ -0,0 +1,150 @@ +require "../../helper" + +module PlaceOS::Api + describe SignageAIProviders do + base = SignageAIProviders.base_route + + # `Spec.before_each` registers against the root context and would run for + # every example in the suite. This hook has to stay local, because turning + # live connections off suite wide would break the specs that talk to core. + before_each do + Model::SignageAIJob.clear + Model::SignageAIProvider.clear + WebMock.allow_net_connect = false + end + + it "lets a sys_admin add a provider without ever echoing the credentials" do + authority = Model::Authority.find_by_domain("localhost").not_nil! + + body = { + name: "openai-#{random_name}", + provider: "OPENAI", + authority_id: authority.id, + credentials: {api_key: "sk-spec-secret"}, + default_model: "gpt-image-2", + is_default: true, + }.to_json + + result = client.post(base, headers: Spec::Authentication.headers, body: body) + + result.status_code.should eq 201 + result.body.should_not contain "credentials" + result.body.should_not contain "sk-spec-secret" + + rendered = JSON.parse(result.body) + rendered["provider"].as_s.should eq "OPENAI" + rendered["is_default"].as_bool.should be_true + + row = Model::SignageAIProvider.find!(UUID.new(rendered["id"].as_s)) + row.credentials_encrypted?.should be_true + row.credentials_json["api_key"].as_s.should eq "sk-spec-secret" + end + + it "keeps the stored credentials when an update leaves them out" do + authority = Model::Authority.find_by_domain("localhost").not_nil! + provider = Model::Generator.signage_ai_provider( + authority: authority, + name: "openai-#{random_name}", + credentials: %({"api_key":"keep-me"}), + ).save! + + result = client.patch( + File.join(base, provider.id.to_s), + headers: Spec::Authentication.headers, + body: {name: "renamed", enabled: false}.to_json, + ) + + result.status_code.should eq 200 + result.body.should_not contain "keep-me" + + row = Model::SignageAIProvider.find!(provider.id.as(UUID)) + row.name.should eq "renamed" + row.enabled.should be_false + row.credentials_json["api_key"].as_s.should eq "keep-me" + end + + it "lets support read, and nothing more" do + authority = Model::Authority.find_by_domain("localhost").not_nil! + provider = Model::Generator.signage_ai_provider( + authority: authority, + name: "openai-#{random_name}", + ).save! + headers = Spec::Authentication.headers(sys_admin: false, support: true) + + index = client.get(base, headers: headers) + index.status_code.should eq 200 + index.body.should_not contain "credentials" + JSON.parse(index.body).as_a.map(&.["id"].as_s).should contain provider.id.to_s + + show = client.get(File.join(base, provider.id.to_s), headers: headers) + show.status_code.should eq 200 + show.body.should_not contain "api_key" + + create = client.post( + base, + headers: headers, + body: {name: "nope", credentials: {api_key: "k"}}.to_json, + ) + create.status_code.should eq 403 + + destroy = client.delete(File.join(base, provider.id.to_s), headers: headers) + destroy.status_code.should eq 403 + Model::SignageAIProvider.find?(provider.id.as(UUID)).should_not be_nil + end + + it "keeps a regular user out entirely" do + authority = Model::Authority.find_by_domain("localhost").not_nil! + provider = Model::Generator.signage_ai_provider( + authority: authority, + name: "openai-#{random_name}", + ).save! + _, headers = Spec::Authentication.authentication(sys_admin: false, support: false) + + client.get(base, headers: headers).status_code.should eq 403 + client.get(File.join(base, provider.id.to_s), headers: headers).status_code.should eq 403 + + create = client.post( + base, + headers: headers, + body: {name: "nope", credentials: {api_key: "k"}}.to_json, + ) + create.status_code.should eq 403 + end + + describe "the credentials test" do + it "reports a working provider" do + authority = Model::Authority.find_by_domain("localhost").not_nil! + provider = Model::Generator.signage_ai_provider( + authority: authority, + name: "openai-#{random_name}", + ).save! + + HttpMocks.signage_ai_vendor(candidates: 1) + + result = client.post(File.join(base, provider.id.to_s, "test"), headers: Spec::Authentication.headers) + result.status_code.should eq 200 + + body = JSON.parse(result.body) + body["ok"].as_bool.should be_true + body["model"].as_s.should eq "gpt-image-2" + end + + it "reports a refusal as a moderation failure rather than an error" do + authority = Model::Authority.find_by_domain("localhost").not_nil! + provider = Model::Generator.signage_ai_provider( + authority: authority, + name: "openai-#{random_name}", + ).save! + + HttpMocks.signage_ai_vendor_refused + + result = client.post(File.join(base, provider.id.to_s, "test"), headers: Spec::Authentication.headers) + result.status_code.should eq 200 + + body = JSON.parse(result.body) + body["ok"].as_bool.should be_false + body["kind"].as_s.should eq "moderation" + end + end + end +end diff --git a/spec/controllers/signage/signage_ai_spec.cr b/spec/controllers/signage/signage_ai_spec.cr new file mode 100644 index 00000000..953b8f0a --- /dev/null +++ b/spec/controllers/signage/signage_ai_spec.cr @@ -0,0 +1,482 @@ +require "../../helper" + +module PlaceOS::Api + # Everything a signage AI request needs: somewhere to put the image, and a + # vendor to ask for one. Returns (authority, storage, provider). + def self.setup_signage_ai(quotas : Hash(String, JSON::Any) = {} of String => JSON::Any) + authority = Model::Authority.find_by_domain("localhost").not_nil! + storage = Model::Generator.storage(authority_id: authority.id.as(String)).save! + provider = Model::Generator.signage_ai_provider( + authority: authority, + name: "openai-#{random_name}", + is_default: true, + quotas: quotas, + ).save! + + {authority, storage, provider} + end + + # A signage group the caller may act in, hung off the authority root (an + # authority only ever has one root group). + def self.signage_group(authority, user, permissions : Model::Permissions) + root = Model::Generator.group(authority: authority).save! + group = Model::Generator.group(authority: authority, parent: root, subsystems: ["signage"]).save! + Model::Generator.group_user(user: user, group: group, permissions: permissions).save! + group + end + + # Drive the long poll at `wait=0` until the job stops moving. The runner is a + # fiber in this same process, so each poll gives it a turn. + def self.await_signage_ai_job(base : String, id : String, headers : HTTP::Headers) : JSON::Any + path = File.join(base, "jobs", id) + + 100.times do + response = client.get(path, headers: headers) + response.status_code.should eq 200 + body = JSON.parse(response.body) + return body if {"done", "failed", "cancelled"}.includes?(body["state"].as_s) + sleep 100.milliseconds + end + + raise "signage AI job #{id} never reached a final state" + end + + describe SignageAI do + base = SignageAI.base_route + + # `Spec.before_each` registers against the root context and would run for + # every example in the suite. This hook has to stay local, because turning + # live connections off suite wide would break the specs that talk to core. + before_each do + Model::SignageAIJob.clear + Model::SignageAIProvider.clear + Model::Playlist::Item.clear + Model::Upload.clear + Model::Storage.clear + clear_group_tables + + # an unstubbed vendor or bucket call should fail loudly rather than reach + # the internet: `HttpMocks.reset` allows real connections by default + WebMock.allow_net_connect = false + end + + describe "capabilities" do + it "is off, with a reason, when the domain has no provider" do + authority = Model::Authority.find_by_domain("localhost").not_nil! + Model::Generator.storage(authority_id: authority.id.as(String)).save! + + result = client.get(File.join(base, "capabilities"), headers: Spec::Authentication.headers) + result.status_code.should eq 200 + + body = JSON.parse(result.body) + body["enabled"].as_bool.should be_false + body["reason"].as_s.should eq "no AI provider is configured for this domain" + body["providers"].as_a.should be_empty + end + + it "is on, and reports what is left of the quota" do + _, _, provider = setup_signage_ai(quotas: {"user_per_day" => JSON::Any.new(5_i64)}) + user, headers = Spec::Authentication.authentication + + # two candidates already spent today + Model::Generator.signage_ai_job(user: user, provider: provider, candidates: 2).save! + + result = client.get(File.join(base, "capabilities"), headers: headers) + result.status_code.should eq 200 + + body = JSON.parse(result.body) + body["enabled"].as_bool.should be_true + body["default_provider_id"].as_s.should eq provider.id.to_s + body["aspect_ratios"].as_a.map(&.as_s).should contain "16:9" + body["quota"]["user_remaining_today"].as_i.should eq 3 + + providers = body["providers"].as_a + providers.size.should eq 1 + providers.first["provider"].as_s.should eq "OPENAI" + providers.first["models"].as_a.map(&.["id"].as_s).should contain "gpt-image-2" + end + end + + describe "generate" do + it "accepts the request, stores the candidate and finishes the job" do + _, storage, _ = setup_signage_ai + user, headers = Spec::Authentication.authentication + + HttpMocks.signage_ai_vendor + HttpMocks.signage_ai_storage + + result = client.post( + File.join(base, "generate"), + headers: headers, + body: {prompt: "a poster for the office party", candidates: 1}.to_json, + ) + + result.status_code.should eq 202 + accepted = JSON.parse(result.body) + accepted["state"].as_s.should eq "queued" + accepted["kind"].as_s.should eq "generate" + accepted["provider"].as_s.should eq "OPENAI" + accepted["model"].as_s.should eq "gpt-image-2" + + job_id = accepted["id"].as_s + Model::SignageAIJob.find!(UUID.new(job_id)).candidates.should eq 1 + + final = await_signage_ai_job(base, job_id, headers) + final["state"].as_s.should eq "done" + final["images_produced"].as_i.should eq 1 + + image = final["images"].as_a.first + image["state"].as_s.should eq "done" + image["width"].as_i.should eq 1 + image["mime"].as_s.should eq "image/jpeg" + + upload = Model::Upload.find!(image["upload_id"].as_s) + upload.uploaded_by.should eq user.id + upload.upload_complete.should be_true + upload.public.should be_false + upload.storage_id.should eq storage.id + upload.tags.should contain ImageGen::Store::CANDIDATE_TAG + upload.tags.should contain "ai-job-#{job_id}" + end + + it "replays an accepted submission rather than spending again" do + setup_signage_ai + user, headers = Spec::Authentication.authentication + + HttpMocks.signage_ai_vendor + HttpMocks.signage_ai_storage + + body = {prompt: "a poster", candidates: 1, idempotency_key: "spec-#{random_name}"}.to_json + + first = client.post(File.join(base, "generate"), headers: headers, body: body) + first.status_code.should eq 202 + job_id = JSON.parse(first.body)["id"].as_s + + second = client.post(File.join(base, "generate"), headers: headers, body: body) + second.status_code.should eq 202 + JSON.parse(second.body)["id"].as_s.should eq job_id + + Model::SignageAIJob.where(user_id: user.id.as(String)).to_a.size.should eq 1 + + # let the runner finish so it hands its slot back + await_signage_ai_job(base, job_id, headers) + end + + it "refuses a non-support caller who names no group" do + authority, _, _ = setup_signage_ai + user, headers = Spec::Authentication.authentication(sys_admin: false, support: false) + signage_group(authority, user, Model::Permissions::Read | Model::Permissions::Create) + + result = client.post( + File.join(base, "generate"), + headers: headers, + body: {prompt: "a poster", candidates: 1}.to_json, + ) + + result.status_code.should eq 403 + Model::SignageAIJob.where(user_id: user.id.as(String)).to_a.should be_empty + end + + it "refuses a caller with only Read on the group" do + authority, _, _ = setup_signage_ai + user, headers = Spec::Authentication.authentication(sys_admin: false, support: false) + group = signage_group(authority, user, Model::Permissions::Read) + + result = client.post( + File.join(base, "generate"), + headers: headers, + body: {prompt: "a poster", candidates: 1, group_id: group.id}.to_json, + ) + + result.status_code.should eq 403 + end + + it "refuses a group that is not in the signage subsystem" do + authority, _, _ = setup_signage_ai + user, headers = Spec::Authentication.authentication(sys_admin: false, support: false) + + root = Model::Generator.group(authority: authority).save! + group = Model::Generator.group(authority: authority, parent: root).save! + permissions = Model::Permissions::Read | Model::Permissions::Create + Model::Generator.group_user(user: user, group: group, permissions: permissions).save! + + result = client.post( + File.join(base, "generate"), + headers: headers, + body: {prompt: "a poster", candidates: 1, group_id: group.id}.to_json, + ) + + result.status_code.should eq 403 + end + + it "answers 429 once the caller has spent their day's allowance" do + setup_signage_ai(quotas: {"user_per_day" => JSON::Any.new(1_i64)}) + _, headers = Spec::Authentication.authentication + + result = client.post( + File.join(base, "generate"), + headers: headers, + body: {prompt: "a poster", candidates: 2}.to_json, + ) + + result.status_code.should eq 429 + JSON.parse(result.body)["kind"].as_s.should eq "quota" + end + + it "records a vendor refusal against the job as a moderation failure" do + setup_signage_ai + _, headers = Spec::Authentication.authentication + + HttpMocks.signage_ai_vendor_refused + HttpMocks.signage_ai_storage + + result = client.post( + File.join(base, "generate"), + headers: headers, + body: {prompt: "a photograph of a real person", candidates: 1}.to_json, + ) + + result.status_code.should eq 202 + + final = await_signage_ai_job(base, JSON.parse(result.body)["id"].as_s, headers) + final["state"].as_s.should eq "failed" + final["error_kind"].as_s.should eq "moderation" + final["images_produced"].as_i.should eq 0 + end + + it "refuses a write with a read-only scope" do + setup_signage_ai + _, scoped_headers = Spec::Authentication.authentication( + scope: [Model::UserJWT::Scope.new("signage_ai", :read)], + ) + + result = client.post( + File.join(base, "generate"), + headers: scoped_headers, + body: {prompt: "a poster", candidates: 1}.to_json, + ) + + result.status_code.should eq 403 + end + end + + describe "edit" do + it "refuses a source the caller neither owns nor can reach through an item" do + authority, storage, _ = setup_signage_ai + user, headers = Spec::Authentication.authentication(sys_admin: false, support: false) + group = signage_group(authority, user, Model::Permissions::Read | Model::Permissions::Create) + + owner = Model::Generator.user(authority: authority).save! + upload = Model::Generator.upload(uploader: owner, storage_id: storage.id).save! + + # the item exists, but it is linked to no group the caller belongs to + item = Model::Generator.item(authority: authority, media_id: upload.id).save! + + result = client.post( + File.join(base, "edit"), + headers: headers, + body: { + prompt: "make it warmer", + candidates: 1, + group_id: group.id, + source_upload_id: upload.id, + source_item_id: item.id, + }.to_json, + ) + + result.status_code.should eq 403 + JSON.parse(result.body)["kind"].as_s.should eq "permission" + end + end + + describe "the long poll" do + it "returns as soon as the version moves" do + _, _, provider = setup_signage_ai + user, headers = Spec::Authentication.authentication + job = Model::Generator.signage_ai_job(user: user, provider: provider).save! + job_id = job.id.as(UUID) + + spawn do + sleep 700.milliseconds + Model::SignageAIJob.bump_version(job_id) + end + + started = Time.utc + result = client.get( + File.join(base, "jobs", job_id.to_s) + "?wait=20&since=#{job.version}", + headers: headers, + ) + elapsed = Time.utc - started + + result.status_code.should eq 200 + JSON.parse(result.body)["version"].as_i.should eq job.version + 1 + elapsed.should be < 10.seconds + end + + it "returns after the wait when nothing moves" do + _, _, provider = setup_signage_ai + user, headers = Spec::Authentication.authentication + job = Model::Generator.signage_ai_job(user: user, provider: provider).save! + + started = Time.utc + result = client.get(File.join(base, "jobs", job.id.to_s) + "?wait=1", headers: headers) + elapsed = Time.utc - started + + result.status_code.should eq 200 + JSON.parse(result.body)["version"].as_i.should eq job.version + elapsed.should be >= 900.milliseconds + end + + it "hides a job belonging to another domain" do + setup_signage_ai + _, headers = Spec::Authentication.authentication + + other = Model::Generator.authority(domain: "ai-spec-#{random_name}.example.com").save! + job = Model::Generator.signage_ai_job(authority: other).save! + + result = client.get(File.join(base, "jobs", job.id.to_s), headers: headers) + result.status_code.should eq 404 + + other.destroy + end + end + + describe "jobs" do + it "lists only the caller's own jobs" do + authority, _, provider = setup_signage_ai + user, headers = Spec::Authentication.authentication + + mine = Model::Generator.signage_ai_job(user: user, provider: provider).save! + someone_else = Model::Generator.user(authority: authority).save! + theirs = Model::Generator.signage_ai_job(user: someone_else, provider: provider).save! + + result = client.get(File.join(base, "jobs") + "?mine=true", headers: headers) + result.status_code.should eq 200 + + ids = JSON.parse(result.body).as_a.map(&.["id"].as_s) + ids.should contain mine.id.to_s + ids.should_not contain theirs.id.to_s + end + + it "only lets support see the whole domain" do + setup_signage_ai + _, headers = Spec::Authentication.authentication(sys_admin: false, support: false) + + result = client.get(File.join(base, "jobs") + "?mine=false", headers: headers) + result.status_code.should eq 403 + end + end + + describe "cancel" do + it "flags a job the caller owns" do + _, _, provider = setup_signage_ai + user, headers = Spec::Authentication.authentication + job = Model::Generator.signage_ai_job(user: user, provider: provider).save! + + result = client.post(File.join(base, "jobs", job.id.to_s, "cancel"), headers: headers) + result.status_code.should eq 200 + JSON.parse(result.body)["version"].as_i.should eq job.version + 1 + + stored = Model::SignageAIJob.find!(job.id.as(UUID)) + stored.cancel_requested.should be_true + end + + it "refuses somebody else's job" do + authority, _, provider = setup_signage_ai + _, headers = Spec::Authentication.authentication(sys_admin: false, support: false) + + someone_else = Model::Generator.user(authority: authority).save! + job = Model::Generator.signage_ai_job(user: someone_else, provider: provider).save! + + result = client.post(File.join(base, "jobs", job.id.to_s, "cancel"), headers: headers) + result.status_code.should eq 403 + Model::SignageAIJob.find!(job.id.as(UUID)).cancel_requested.should be_false + end + end + + describe "claim" do + it "marks the candidate kept and records the item on the job" do + _, storage, provider = setup_signage_ai + user, headers = Spec::Authentication.authentication + + upload = Model::Generator.upload(uploader: user, storage_id: storage.id).save! + upload.tags = [ImageGen::Store::CANDIDATE_TAG, "ai-job-spec"] + upload.save! + + item = Model::Generator.item(media_id: upload.id).save! + + job = Model::Generator.signage_ai_job(user: user, provider: provider, candidates: 1) + job.state = Model::SignageAIJob::State::Done + job.result = JSON.parse({images: [{state: "done", index: 0, upload_id: upload.id}]}.to_json) + job.save! + + result = client.post( + File.join(base, "jobs", job.id.to_s, "claim"), + headers: headers, + body: {upload_id: upload.id, item_id: item.id}.to_json, + ) + + result.status_code.should eq 200 + JSON.parse(result.body)["images"].as_a.first["item_id"].as_s.should eq item.id + + kept = Model::Upload.find!(upload.id.as(String)) + kept.tags.should contain "ai-claimed" + kept.tags.should_not contain ImageGen::Store::CANDIDATE_TAG + end + + it "refuses an item that does not use the image" do + _, storage, provider = setup_signage_ai + user, headers = Spec::Authentication.authentication + + upload = Model::Generator.upload(uploader: user, storage_id: storage.id).save! + unrelated = Model::Generator.upload(uploader: user, storage_id: storage.id).save! + item = Model::Generator.item(media_id: unrelated.id).save! + + job = Model::Generator.signage_ai_job(user: user, provider: provider, candidates: 1) + job.state = Model::SignageAIJob::State::Done + job.result = JSON.parse({images: [{state: "done", index: 0, upload_id: upload.id}]}.to_json) + job.save! + + result = client.post( + File.join(base, "jobs", job.id.to_s, "claim"), + headers: headers, + body: {upload_id: upload.id, item_id: item.id}.to_json, + ) + + result.status_code.should eq 422 + end + end + + describe "usage" do + it "sums spend per provider and model" do + _, _, provider = setup_signage_ai + user, headers = Spec::Authentication.authentication + + job = Model::Generator.signage_ai_job(user: user, provider: provider, candidates: 2) + job.state = Model::SignageAIJob::State::Done + job.images_produced = 2 + job.cost_units = 12.5 + job.save! + + result = client.get(File.join(base, "usage"), headers: headers) + result.status_code.should eq 200 + + rows = Array(Model::SignageAIJob::UsageRow).from_json(result.body) + row = rows.find { |entry| entry.provider == "OPENAI" }.not_nil! + row.model.should eq "gpt-image-2" + row.jobs.should eq 1 + row.candidates.should eq 2 + row.images_produced.should eq 2 + row.cost_units.should eq 12.5 + end + + it "is support only" do + setup_signage_ai + _, headers = Spec::Authentication.authentication(sys_admin: false, support: false) + + result = client.get(File.join(base, "usage"), headers: headers) + result.status_code.should eq 403 + end + end + end +end diff --git a/spec/helper.cr b/spec/helper.cr index 08027b49..489ddfb0 100644 --- a/spec/helper.cr +++ b/spec/helper.cr @@ -48,6 +48,11 @@ require "placeos-models/spec/generator" PgORM::Database.configure { |_| } def clear_tables + # Neither signage AI model is a ModelBase, so nothing else picks them up. + # Jobs FK into providers, so they go first. + PlaceOS::Model::SignageAIJob.clear + PlaceOS::Model::SignageAIProvider.clear + # Clear group-system tables first — they FK into User/Authority/Zone # and must be dropped before their parents. Done sequentially since # some of them reference each other. diff --git a/spec/spec_helpers/http_mocks.cr b/spec/spec_helpers/http_mocks.cr index 12996621..b11bf068 100644 --- a/spec/spec_helpers/http_mocks.cr +++ b/spec/spec_helpers/http_mocks.cr @@ -1,6 +1,12 @@ +require "base64" require "webmock" module PlaceOS::Api::HttpMocks + # A real 1x1 JPEG. The signage AI runner reads the mime type and the + # dimensions straight out of the file header, so a vendor stub has to answer + # with an image rather than arbitrary bytes. + TINY_JPEG = "/9j/4AAQSkZJRgABAQEAYABgAAD/2wBDAAgGBgcGBQgHBwcJCQgKDBQNDAsLDBkSEw8UHRofHh0aHBwgJC4nICIsIxwcKDcpLDAxNDQ0Hyc5PTgyPC4zNDL/wAALCAABAAEBAREA/8QAFAABAAAAAAAAAAAAAAAAAAAACf/EABQQAQAAAAAAAAAAAAAAAAAAAAD/2gAIAQEAAD8AKp//2Q==" + def self.reset WebMock.reset WebMock.allow_net_connect = true @@ -37,4 +43,59 @@ module PlaceOS::Api::HttpMocks HTTP::Client::Response.new(200, body, headers) end end + + # OpenAI's image API, answering with `TINY_JPEG`. `candidates` pins the number + # of images returned; left out, the stub returns as many as the adapter asked + # for, which is how the vendor behaves. + # + # Only the first matching stub is used, so a spec that wants a refusal must + # register `signage_ai_vendor_refused` and not this one. + def self.signage_ai_vendor(candidates : Int32? = nil) + WebMock + .stub(:post, /api\.openai\.com/) + .to_return do |request| + wanted = candidates + if wanted.nil? + wanted = begin + JSON.parse(WebMock.body(request).to_s)["n"]?.try(&.as_i?) || 1 + rescue JSON::ParseException + 1 + end + end + + body = { + data: Array.new(wanted) { {b64_json: TINY_JPEG} }, + usage: {total_tokens: 10 * wanted}, + }.to_json + + HTTP::Client::Response.new(200, body, HTTP::Headers{"Content-Type" => "application/json"}) + end + end + + # The vendor blocking a prompt, which the adapter maps to `Moderated`. + def self.signage_ai_vendor_refused + body = {error: {code: "content_policy_violation", message: "the request was blocked"}}.to_json + + WebMock + .stub(:post, /api\.openai\.com/) + .to_return(body: body, status: 400, headers: HTTP::Headers{"Content-Type" => "application/json"}) + end + + # The object store a generated candidate is written to and read back from. + # Both verbs are signed URLs against the storage provider's host. + def self.signage_ai_storage + object_store = /amazonaws\.com|blob\.core\.windows\.net/ + + WebMock.stub(:put, object_store).to_return(body: "", status: 200) + + WebMock + .stub(:get, object_store) + .to_return do |_request| + HTTP::Client::Response.new( + 200, + String.new(::Base64.decode(TINY_JPEG)), + HTTP::Headers{"Content-Type" => "image/jpeg"}, + ) + end + end end diff --git a/src/placeos-rest-api/utilities/image_gen/adapters/gemini_vertex.cr b/src/placeos-rest-api/utilities/image_gen/adapters/gemini_vertex.cr index 58d2a54a..4fbe90f7 100644 --- a/src/placeos-rest-api/utilities/image_gen/adapters/gemini_vertex.cr +++ b/src/placeos-rest-api/utilities/image_gen/adapters/gemini_vertex.cr @@ -196,7 +196,7 @@ module PlaceOS::Api::ImageGen::Adapters [AdapterImage.new( bytes: bytes, - mime: inline["mimeType"]?.try(&.as_s?) || "image/jpeg", + mime: Http.mime_of(bytes, inline["mimeType"]?.try(&.as_s?) || "image/jpeg"), width: dimensions.try(&.[0]), height: dimensions.try(&.[1]), vendor_id: payload["responseId"]?.try(&.as_s?), diff --git a/src/placeos-rest-api/utilities/image_gen/adapters/openai_images.cr b/src/placeos-rest-api/utilities/image_gen/adapters/openai_images.cr index 9881861a..29ce2272 100644 --- a/src/placeos-rest-api/utilities/image_gen/adapters/openai_images.cr +++ b/src/placeos-rest-api/utilities/image_gen/adapters/openai_images.cr @@ -182,7 +182,7 @@ module PlaceOS::Api::ImageGen::Adapters dimensions = Http.dimensions(bytes) AdapterImage.new( bytes: bytes, - mime: "image/jpeg", + mime: Http.mime_of(bytes), width: dimensions.try(&.[0]), height: dimensions.try(&.[1]), cost_units: per_image, diff --git a/src/placeos-rest-api/utilities/image_gen/http.cr b/src/placeos-rest-api/utilities/image_gen/http.cr index b3383a91..b037f9b7 100644 --- a/src/placeos-rest-api/utilities/image_gen/http.cr +++ b/src/placeos-rest-api/utilities/image_gen/http.cr @@ -43,6 +43,16 @@ module PlaceOS::Api::ImageGen raise Error::ImageGen::Vendor.new("too many redirects reading image") end + # What the bytes actually are, rather than what the request asked for. A + # gateway in front of a vendor may transcode, and the stored object's + # content type has to match what a browser will be served. + def self.mime_of(bytes : Bytes, fallback : String = "image/jpeg") : String + return "image/png" if bytes.size > 8 && bytes[0] == 0x89 && bytes[1] == 0x50 + return "image/jpeg" if bytes.size > 3 && bytes[0] == 0xFF && bytes[1] == 0xD8 + return "image/webp" if bytes.size > 12 && bytes[8] == 0x57 && bytes[9] == 0x45 && bytes[10] == 0x42 && bytes[11] == 0x50 + fallback + end + # Width and height straight out of the file header. The api binary is built # `--static` from scratch and has no image library, and we only ever deal # with JPEG and PNG here. diff --git a/src/placeos-rest-api/utilities/image_gen/runner.cr b/src/placeos-rest-api/utilities/image_gen/runner.cr index f85371ce..2fc7987b 100644 --- a/src/placeos-rest-api/utilities/image_gen/runner.cr +++ b/src/placeos-rest-api/utilities/image_gen/runner.cr @@ -30,9 +30,18 @@ module PlaceOS::Api::ImageGen user_id: context.user.id, ) + # Slots were reserved by the request handler, one per planned vendor call. + # This is the only thing that hands them back, so every exit path has to + # drain the ledger. Atomic because the candidate fibers decrement it as + # they finish, and `swap` makes each drain a one time claim. + held = Atomic(Int32).new(context.adapter.calls_for(context.request.candidates)) + started = Time.utc job = ::PlaceOS::Model::SignageAIJob.find?(context.job_id) - return if job.nil? + if job.nil? + release(held.swap(0)) + return + end job.state = ::PlaceOS::Model::SignageAIJob::State::Running job.started_at = started @@ -84,6 +93,7 @@ module PlaceOS::Api::ImageGen ensure # one release per finished vendor call ImageGen.slots.release + held.sub(1) done.send(nil) end end @@ -91,11 +101,20 @@ module PlaceOS::Api::ImageGen calls.size.times { done.receive } + release(held.swap(0)) + finish(context, produced, cost, failure, started) rescue ex + # a failure before or between the candidate fibers: the ones that ran gave + # their slot back in an ensure, the rest never will, so square the ledger Log.error(exception: ex) { "signage AI job failed outside a vendor call" } fail_job(context.job_id, ex) - # nothing reserved past this point, but a partial plan may still hold slots + release(held.swap(0)) if held + end + + # hand back slots this run still holds, never more + private def self.release(count : Int32) : Nil + count.times { ImageGen.slots.release } if count > 0 end # Fetch the source and reference bytes once, before any vendor call. diff --git a/src/placeos-rest-api/utilities/image_gen/store.cr b/src/placeos-rest-api/utilities/image_gen/store.cr index 124637ff..5e6c464b 100644 --- a/src/placeos-rest-api/utilities/image_gen/store.cr +++ b/src/placeos-rest-api/utilities/image_gen/store.cr @@ -104,10 +104,7 @@ module PlaceOS::Api::ImageGen bytes, mime = Http.get_bytes(url) # trust the file header over whatever the bucket reported - mime = "image/png" if bytes.size > 8 && bytes[0] == 0x89 && bytes[1] == 0x50 - mime = "image/jpeg" if bytes.size > 3 && bytes[0] == 0xFF && bytes[1] == 0xD8 - - Reference.new(bytes: bytes, mime: mime) + Reference.new(bytes: bytes, mime: Http.mime_of(bytes, mime)) end end end From b21550bf73b8adb0bf39382d5561c251069fcfa1 Mon Sep 17 00:00:00 2001 From: Cameron Reeves Date: Fri, 28 Aug 2026 09:01:22 +1000 Subject: [PATCH 03/14] fix(signage-ai): ask for the words when the model is drawing them Turning off the text layer means the model produces the finished poster, but the prompt still asked for a background and never mentioned the wording, so the words never appeared. It now asks for a poster rather than a background, and for the brief's wording to be rendered legibly. Found running the flow in a browser with the toggle off. Co-Authored-By: Claude Opus 5 (1M context) --- src/placeos-rest-api/utilities/image_gen/prompt.cr | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/placeos-rest-api/utilities/image_gen/prompt.cr b/src/placeos-rest-api/utilities/image_gen/prompt.cr index f438b89a..d70abbac 100644 --- a/src/placeos-rest-api/utilities/image_gen/prompt.cr +++ b/src/placeos-rest-api/utilities/image_gen/prompt.cr @@ -97,16 +97,25 @@ module PlaceOS::Api::ImageGen else "Landscape" end - parts = ["#{orientation} #{options.aspect} poster background for a digital signage screen."] + parts = [] of String case options.text_mode in TextMode::Layer + # the app draws the words over the top, so the model is asked for a + # background and told to keep the lettering out of it + parts << "#{orientation} #{options.aspect} poster background for a digital signage screen." parts << "Leave the top third clear and plain so a headline can be added over it." parts << "Leave a clear area in the bottom right corner for a logo." if options.include_logo parts << "Do not render any text, letters, numbers, words or logos anywhere in the image." in TextMode::Model + # the caller opted out of the text layer, so the model has to produce a + # finished poster. Without this it is asked for a background and answers + # with one, and the words never appear. + parts << "#{orientation} #{options.aspect} poster for a digital signage screen." if (words = options.words) && words.presence - parts << "Render exactly this text and nothing else: #{words.inspect}." + parts << "Render exactly this text, spelt correctly and large enough to read across a room: #{words.inspect}." + else + parts << "Render the wording from the brief as part of the poster, spelt correctly and large enough to read across a room." end parts << "Leave a clear area in the bottom right corner for a logo." if options.include_logo end From 61b561f9ee69e6f49d957c72b2a93023faa1cdad Mon Sep 17 00:00:00 2001 From: Cameron Reeves Date: Fri, 28 Aug 2026 09:39:24 +1000 Subject: [PATCH 04/14] fix(signage-ai): let a provider's endpoint be cleared A field left out of the body is left alone, but a field sent empty now clears the stored value. Without the second half an endpoint could be set and never unset, and an empty endpoint is exactly what sends a provider to the vendor's own host rather than to a gateway: a row pointed at a test endpoint could not be moved to the real one through the API. Co-Authored-By: Claude Opus 5 (1M context) --- .../controllers/signage/ai_providers.cr | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/placeos-rest-api/controllers/signage/ai_providers.cr b/src/placeos-rest-api/controllers/signage/ai_providers.cr index ffc45369..f2480bc3 100644 --- a/src/placeos-rest-api/controllers/signage/ai_providers.cr +++ b/src/placeos-rest-api/controllers/signage/ai_providers.cr @@ -176,10 +176,14 @@ module PlaceOS::Api raise Error::ModelValidation.new([Error::Field.new(:provider, "must be one of OPENAI, AZURE_OPENAI, GOOGLE_VERTEX")]) end + # A field left out of the body is left alone; a field sent empty is cleared. + # Without the second half an endpoint could be set but never unset, which + # matters because an empty endpoint is what sends a provider to the vendor's + # own host rather than to a gateway. private def apply_optional(row : ::PlaceOS::Model::SignageAIProvider, params : ProviderParams) : Nil - row.endpoint = params.endpoint if params.endpoint - row.location = params.location if params.location - row.default_model = params.default_model if params.default_model + row.endpoint = params.endpoint.try(&.presence) if params.endpoint + row.location = params.location.try(&.presence) if params.location + row.default_model = params.default_model.try(&.presence) if params.default_model row.allowed_models = params.allowed_models.not_nil! if params.allowed_models row.enabled = params.enabled.not_nil! unless params.enabled.nil? row.is_default = params.is_default.not_nil! unless params.is_default.nil? From 9886c5627b49b14effeca3d73260ce5dab765f6c Mon Sep 17 00:00:00 2001 From: Cameron Reeves Date: Fri, 28 Aug 2026 10:01:58 +1000 Subject: [PATCH 05/14] feat(signage-ai): art direction against the generated look, and a cheaper credentials check Image models left alone produce a recognisable house style: centred title over a purple gradient, glowing orbs, floating geometry. Every request now carries a counter-brief ahead of the brand kit and the brief. Most of it applies whatever draws the words. Four lines do not: when the app composites the headline afterwards the model is producing a background, so art directing typography, asking it not to place text over a background, and telling it to reproduce supplied wording all argue with the layout instruction that follows. Those are held back in that mode and specced both ways. Also fixes the credentials check, which asked for 2048x2048 at the medium tier: a full price image taking half a minute, for something documented as one small image. It now asks for 1024x1024 at the vendor's cheapest tier, which needed an explicit size override and a draft quality that maps to OpenAI's low. Co-Authored-By: Claude Opus 5 (1M context) --- spec/signage_ai_prompt_spec.cr | 88 +++++++++++++++++ .../controllers/signage/ai_providers.cr | 7 ++ .../image_gen/adapters/openai_images.cr | 6 +- .../utilities/image_gen/prompt.cr | 96 +++++++++++++++++++ .../utilities/image_gen/types.cr | 7 +- 5 files changed, 201 insertions(+), 3 deletions(-) create mode 100644 spec/signage_ai_prompt_spec.cr diff --git a/spec/signage_ai_prompt_spec.cr b/spec/signage_ai_prompt_spec.cr new file mode 100644 index 00000000..9e20dece --- /dev/null +++ b/spec/signage_ai_prompt_spec.cr @@ -0,0 +1,88 @@ +require "./helper" + +module PlaceOS::Api + describe ImageGen::Prompt do + brand = ImageGen::Prompt::BrandKit.parse(JSON.parse(%({ + "organisation": "Acme", + "palette": {"primary": "#0E6E52"}, + "tone": "warm, professional", + "never_include": ["competitor logos"] + }))) + + describe "art direction" do + it "sends the counter-brief ahead of the brief" do + prompt = ImageGen::Prompt.build(ImageGen::Prompt::Options.new( + brief: "a poster for the office party", + aspect: "16:9", + )) + + prompt.should contain "Avoid the generic" + prompt.should contain "AVOID:" + prompt.should contain "purple-blue-orange gradients" + # the style has to land before the brief, not after it + prompt.index("Avoid the generic").not_nil!.should be < prompt.index("Brief:").not_nil! + end + + it "holds back the typography lines when the app is setting the type" do + prompt = ImageGen::Prompt.build(ImageGen::Prompt::Options.new( + brief: "a poster for the office party", + aspect: "16:9", + text_mode: ImageGen::Prompt::TextMode::Layer, + )) + + # these argue with "leave the top third clear" and with there being no + # supplied wording at all + prompt.should_not contain "Typography should feel professionally art-directed" + prompt.should_not contain "rather than placing text over a generic background" + prompt.should_not contain "reproduce the supplied wording exactly" + + # but the rest of the direction still applies + prompt.should contain "Make the layout feel designed rather than algorithmically balanced" + prompt.should contain "Do not render any text, letters, numbers, words or logos" + end + + it "includes them when the model is setting the type" do + prompt = ImageGen::Prompt.build(ImageGen::Prompt::Options.new( + brief: "a poster for the office party", + aspect: "16:9", + text_mode: ImageGen::Prompt::TextMode::Model, + )) + + prompt.should contain "Typography should feel professionally art-directed" + prompt.should contain "reproduce the supplied wording exactly" + prompt.should_not contain "Do not render any text" + end + + it "stays inside a sane prompt length with a brand kit and a refine chain" do + prompt = ImageGen::Prompt.build(ImageGen::Prompt::Options.new( + brief: "a poster for the office Christmas party on Friday 10 December", + aspect: "16:9", + brand: brand, + history: ["make it warmer", "add paper decorations"], + instruction: "move the tree left", + )) + + # gpt-image-2 accepts far more than this, but a runaway prompt would be + # a cost and a latency problem rather than an error + prompt.size.should be < 6000 + end + end + + it "carries the brand kit, the brief and each change in order" do + prompt = ImageGen::Prompt.build(ImageGen::Prompt::Options.new( + brief: "a poster for the office party", + aspect: "16:9", + brand: brand, + history: ["make it warmer"], + instruction: "add decorations", + )) + + prompt.should contain "Organisation: Acme." + prompt.should contain "primary #0E6E52" + prompt.should contain "Change 1: make it warmer" + prompt.should contain "Change 2: add decorations" + prompt.should contain "Keep everything else exactly as it is." + prompt.should contain "Never include: competitor logos." + end + end +end diff --git a/src/placeos-rest-api/controllers/signage/ai_providers.cr b/src/placeos-rest-api/controllers/signage/ai_providers.cr index f2480bc3..29fc0bd6 100644 --- a/src/placeos-rest-api/controllers/signage/ai_providers.cr +++ b/src/placeos-rest-api/controllers/signage/ai_providers.cr @@ -136,6 +136,10 @@ module PlaceOS::Api current_provider.destroy end + # 1024x1024 is the smallest gpt-image-2 accepts (it wants at least 655,360 + # total pixels, and both edges a multiple of 16) + PROBE_SIZE = "1024x1024" + record TestResult, ok : Bool, latency_ms : Int64, model : String?, error : String? = nil, kind : String? = nil do include JSON::Serializable end @@ -152,6 +156,8 @@ module PlaceOS::Api begin raise Error::ImageGen::NotConfigured.new("no model configured") if model.nil? + # the smallest, cheapest thing the vendor will draw: this only has to + # prove the credentials work, and it is discarded adapter.generate(ImageGen::AdapterRequest.new( kind: ImageGen::Kind::Generate, prompt: "A plain mid grey background. No text, no logos, no objects.", @@ -159,6 +165,7 @@ module PlaceOS::Api quality: "draft", candidates: 1, model: model, + size_override: PROBE_SIZE, )) TestResult.new(ok: true, latency_ms: (Time.utc - started).total_milliseconds.to_i64, model: model) diff --git a/src/placeos-rest-api/utilities/image_gen/adapters/openai_images.cr b/src/placeos-rest-api/utilities/image_gen/adapters/openai_images.cr index 29ce2272..67762b5b 100644 --- a/src/placeos-rest-api/utilities/image_gen/adapters/openai_images.cr +++ b/src/placeos-rest-api/utilities/image_gen/adapters/openai_images.cr @@ -93,7 +93,11 @@ module PlaceOS::Api::ImageGen::Adapters # leaderboards rank first and what iteration runs at. "high" is the # explicit enhance step. private def quality(value : String) : String - value == "high" ? "high" : "medium" + case value + when "high" then "high" + when "draft" then "low" + else "medium" + end end private def azure? : Bool diff --git a/src/placeos-rest-api/utilities/image_gen/prompt.cr b/src/placeos-rest-api/utilities/image_gen/prompt.cr index d70abbac..d20f2034 100644 --- a/src/placeos-rest-api/utilities/image_gen/prompt.cr +++ b/src/placeos-rest-api/utilities/image_gen/prompt.cr @@ -10,6 +10,100 @@ module PlaceOS::Api::ImageGen # file for the same reason, so the prompt asks for room rather than a drawing # of it. module Prompt + # Art direction sent ahead of every brief. + # + # Image models left to themselves produce a recognisable house style: + # centred title over a purple gradient, glowing orbs, floating geometry. + # This is the counter-brief, and it is the largest single part of the + # prompt, so it is kept here rather than buried in a method. + # + # Split because most of it applies whatever draws the words, but a few + # lines only make sense when the model is setting the type. When the app + # composites the headline afterwards the model is producing a background, + # and telling it to art direct typography or to avoid "text over a generic + # background" argues with the layout instruction that follows. + STYLE_OPENING = <<-TEXT + Avoid the generic "AI-generated poster" aesthetic. + Design this as if it were created by an experienced human graphic designer working from a real creative brief, not generated from a text prompt. + TEXT + + STYLE_REQUIREMENTS = <<-TEXT + STYLE REQUIREMENTS: + - Use deliberate, editorial graphic design with a clear visual concept. + - Prioritise composition, spacing, hierarchy and art direction over decorative effects. + - Make the layout feel designed rather than algorithmically balanced. + - Allow asymmetry, negative space, unusual cropping, restrained layouts and imperfect/off-centre placement where appropriate. + - Use a limited, intentional colour palette. + - Aim for the quality of a professionally commissioned event poster, cultural institution campaign, design studio project, magazine advertisement or high-end printed flyer. + - The finished piece should feel plausible as real-world graphic design. + TEXT + + # only when the model is setting the type + STYLE_TYPOGRAPHY = <<-TEXT + - Prioritise strong typography alongside composition and hierarchy. + - Typography should feel professionally art-directed and appropriate to the subject. + - Use no more typefaces, weights or text effects than a competent designer would realistically choose. + - Treat photography/illustration as an integrated part of the composition rather than placing text over a generic background. + TEXT + + STYLE_AVOID = <<-TEXT + AVOID: + - generic AI poster aesthetics + - default futuristic/corporate styling + - purple-blue-orange gradients unless specifically requested + - glowing neon edges + - glowing blobs or orbs + - random floating geometric shapes + - abstract 3D objects added only to fill space + - excessive glassmorphism + - unnecessary lens flares or bloom + - generic particle effects + - holographic effects + - fake depth-of-field added to graphic design + - huge centred title + subtitle + button style layouts + - perfectly symmetrical compositions unless the concept calls for it + - generic vector people or corporate illustrations + - arbitrary decorative squiggles + - excessive rounded rectangles + - random badges, pills or UI components + - meaningless microtext + - fake logos + - pseudo-technical markings + - ornamental elements without a clear design purpose + - overly polished "concept art" rendering + - the appearance of a Canva template + - the appearance of a cryptocurrency, SaaS, Web3 or AI conference poster unless specifically requested + - visual clutter added merely to make the design seem sophisticated + TEXT + + STYLE_CLOSING = <<-TEXT + IMPORTANT: + Do not interpret "professional" as "futuristic", "glossy", "minimal corporate", or "luxury gradient". + Before designing, infer an appropriate real-world graphic-design direction from the subject matter. Establish a specific visual idea and let that idea determine the typography, image treatment, composition and colour palette. + The design should have character and specificity. It should look like somebody made actual aesthetic decisions. + Do not add text, icons, graphics, logos, dates, URLs, QR codes or decorative elements that were not requested. + TEXT + + # only when the model is setting the type: there is no supplied wording to + # reproduce when the app draws it afterwards + STYLE_TEXT_FIDELITY = <<-TEXT + For any required text, reproduce the supplied wording exactly. Do not paraphrase it, invent additional copy, or fill empty areas with placeholder text. + TEXT + + def self.style(text_mode : TextMode) : String + sections = [STYLE_OPENING] of String + + requirements = STYLE_REQUIREMENTS + requirements = "#{requirements}\n#{STYLE_TYPOGRAPHY}" if text_mode.model? + sections << requirements + + sections << STYLE_AVOID + sections << STYLE_CLOSING + sections << STYLE_TEXT_FIDELITY if text_mode.model? + + sections.join("\n") + end + # `signage_ai` metadata on the org zone struct BrandKit include JSON::Serializable @@ -61,6 +155,8 @@ module PlaceOS::Api::ImageGen def self.build(options : Options) : String lines = [] of String + lines << style(options.text_mode) + if (brand = options.brand) parts = [] of String parts << "Organisation: #{brand.organisation}." if brand.organisation.presence diff --git a/src/placeos-rest-api/utilities/image_gen/types.cr b/src/placeos-rest-api/utilities/image_gen/types.cr index 102d9aba..da4bfafa 100644 --- a/src/placeos-rest-api/utilities/image_gen/types.cr +++ b/src/placeos-rest-api/utilities/image_gen/types.cr @@ -21,7 +21,10 @@ module PlaceOS::Api::ImageGen model : String, references : Array(Reference) = [] of Reference, source : Reference? = nil, - options : Hash(String, JSON::Any) = {} of String => JSON::Any do + options : Hash(String, JSON::Any) = {} of String => JSON::Any, + # overrides the size the aspect would give. Only the credentials check uses + # this, to ask for the cheapest thing a vendor will draw. + size_override : String? = nil do def width : Int32 ImageGen.size_for(aspect)[0] end @@ -31,7 +34,7 @@ module PlaceOS::Api::ImageGen end def size : String - "#{width}x#{height}" + size_override || "#{width}x#{height}" end end From 2384fa52109fb16f75261747de758a216d25f137 Mon Sep 17 00:00:00 2001 From: Cameron Reeves Date: Fri, 28 Aug 2026 10:33:11 +1000 Subject: [PATCH 06/14] fix(signage-ai): stop an edit being sent as a redesign brief Editing changed far more than was asked. Three causes, all ours rather than the model's: - A first edit sent the user's words as a *brief*, not an instruction, so the "keep everything else" framing was dropped entirely. It only appeared from the second refine onward. - The generation art direction went out with every edit, including "establish a specific visual idea and let that idea determine the typography, image treatment, composition and colour palette". That is a redesign brief, and it explains the changed fonts and colours exactly. - The layout line went out too, so an edit of a text heavy poster was also told it was making a background and to render no text at all. An edit now gets its own prompt: preservation first, the original brief only as context, each earlier change listed as already applied, and the new one named as the only change to make. None of the generation direction is sent. gpt-image-2 still regenerates the whole frame rather than painting into a region, so this reduces drift rather than removing it. If it is still too loose the fallback is to route edits to Nano Banana 2, which measures best of the current models at preserving unedited regions. Co-Authored-By: Claude Opus 5 (1M context) --- spec/signage_ai_prompt_spec.cr | 49 +++++++++++++++++++ .../controllers/signage/ai.cr | 20 ++++++-- .../utilities/image_gen/prompt.cr | 46 +++++++++++++++++ 3 files changed, 111 insertions(+), 4 deletions(-) diff --git a/spec/signage_ai_prompt_spec.cr b/spec/signage_ai_prompt_spec.cr index 9e20dece..0c5683cc 100644 --- a/spec/signage_ai_prompt_spec.cr +++ b/spec/signage_ai_prompt_spec.cr @@ -68,6 +68,55 @@ module PlaceOS::Api end end + describe "editing" do + it "asks for preservation rather than design" do + prompt = ImageGen::Prompt.build(ImageGen::Prompt::Options.new( + brief: "", + aspect: "9:16", + kind: ImageGen::Kind::Edit, + instruction: %(Make the first "EVENT NAME" text read "PIZZA DAY" instead), + )) + + prompt.should contain "Make only the change described below" + prompt.should contain "every typeface, weight, size, casing and text position" + prompt.should contain "The change to make now, and the only one:" + prompt.should contain %(read "PIZZA DAY") + + # none of the generation brief: it is what asks for a redesign + prompt.should_not contain "Avoid the generic" + prompt.should_not contain "AVOID:" + prompt.should_not contain "let that idea determine the typography" + prompt.should_not contain "poster background for a digital signage screen" + prompt.should_not contain "Do not render any text" + end + + it "keeps the instruction an instruction on a first edit, not a brief" do + # the bug this replaced: with no parent job the words were sent as a + # brief and the change framing was dropped entirely + prompt = ImageGen::Prompt.build(ImageGen::Prompt::Options.new( + brief: "", + aspect: "16:9", + kind: ImageGen::Kind::Edit, + instruction: "swap the date to 12 September", + )) + prompt.should contain "The change to make now, and the only one: swap the date to 12 September" + end + + it "lists changes already applied when refining" do + prompt = ImageGen::Prompt.build(ImageGen::Prompt::Options.new( + brief: "a poster for the office party", + aspect: "16:9", + kind: ImageGen::Kind::Edit, + history: ["make it warmer"], + instruction: "add paper decorations", + )) + + prompt.should contain "It was made for this brief: a poster for the office party" + prompt.should contain "Change 1, already applied: make it warmer" + prompt.should contain "The change to make now, and the only one: add paper decorations" + end + end + it "carries the brand kit, the brief and each change in order" do prompt = ImageGen::Prompt.build(ImageGen::Prompt::Options.new( brief: "a poster for the office party", diff --git a/src/placeos-rest-api/controllers/signage/ai.cr b/src/placeos-rest-api/controllers/signage/ai.cr index 404c3808..f20d727a 100644 --- a/src/placeos-rest-api/controllers/signage/ai.cr +++ b/src/placeos-rest-api/controllers/signage/ai.cr @@ -501,17 +501,29 @@ module PlaceOS::Api reference_ids << logo unless reference_ids.includes?(logo) end - history = parent ? (parent.chain.compact_map(&.prompt) + [parent.prompt].compact) : [] of String + chain = parent ? (parent.chain.compact_map(&.prompt) + [parent.prompt].compact) : [] of String + editing = kind == ImageGen::Kind::Edit + + # An edit's words are always an instruction, never a brief. Sending them + # as a brief on a first edit lost the "change only this" framing, which + # is the one line that keeps the rest of the image still. + original_brief = editing ? chain.first? : (parent ? chain.first? || prompt : prompt) + history = if editing + chain.empty? ? [] of String : chain[1..]? || [] of String + else + parent ? chain[1..]? || [] of String : [] of String + end text = ImageGen::Prompt.build(ImageGen::Prompt::Options.new( - brief: parent ? (history.first? || prompt) : prompt, + brief: original_brief || "", aspect: aspect, + kind: kind, text_mode: text_layer ? ImageGen::Prompt::TextMode::Layer : ImageGen::Prompt::TextMode::Model, include_logo: include_logo, brand: brand_kit, words: words, - history: parent ? history[1..]? || [] of String : [] of String, - instruction: parent ? prompt : nil, + history: history, + instruction: editing || parent ? prompt : nil, )) request = ImageGen::AdapterRequest.new( diff --git a/src/placeos-rest-api/utilities/image_gen/prompt.cr b/src/placeos-rest-api/utilities/image_gen/prompt.cr index d20f2034..1f6a5d11 100644 --- a/src/placeos-rest-api/utilities/image_gen/prompt.cr +++ b/src/placeos-rest-api/utilities/image_gen/prompt.cr @@ -145,6 +145,7 @@ module PlaceOS::Api::ImageGen record Options, brief : String, aspect : String, + kind : Kind = Kind::Generate, text_mode : TextMode = TextMode::Layer, include_logo : Bool = true, brand : BrandKit? = nil, @@ -152,7 +153,30 @@ module PlaceOS::Api::ImageGen history : Array(String) = [] of String, instruction : String? = nil + # What an edit is told, in place of the art direction and the layout brief. + # + # gpt-image-2 regenerates the whole frame rather than painting into a + # region, so an edit drifts unless it is held down hard. The generation + # style block makes that worse: it asks the model to establish a visual + # idea and let it "determine the typography, image treatment, composition + # and colour palette", which is a redesign brief. None of it is sent here. + EDIT_PRESERVATION = <<-TEXT + Edit the supplied image. Make only the change described below and leave everything else untouched. + Preserve exactly, unless the change itself requires otherwise: + - the existing layout, composition, framing and crop + - every colour, including backgrounds, fills and accents + - every typeface, weight, size, casing and text position + - all other text, word for word, including any wording you would consider a placeholder + - all existing photography, illustration, logos and graphic elements + Do not redesign, restyle, recolour, re-typeset, re-crop or re-compose the image. + Do not "improve" anything that was not part of the change. + Do not add text, graphics or decoration that was not asked for. + The result should look like the supplied image with one alteration made to it, not like a new design of the same subject. + TEXT + def self.build(options : Options) : String + return build_edit(options) if options.kind.edit? + lines = [] of String lines << style(options.text_mode) @@ -186,6 +210,28 @@ module PlaceOS::Api::ImageGen lines.join("\n") end + private def self.build_edit(options : Options) : String + lines = [EDIT_PRESERVATION] of String + + # what the poster was originally for, so a change is read in context + lines << "The image is a #{options.aspect} poster for a digital signage screen." + lines << "It was made for this brief: #{options.brief}" if options.brief.presence + + options.history.each_with_index do |entry, index| + lines << "Change #{index + 1}, already applied: #{entry}" + end + + if (instruction = options.instruction) && instruction.presence + lines << "The change to make now, and the only one: #{instruction}" + end + + if (brand = options.brand) && !brand.never_include.empty? + lines << "Never include: #{brand.never_include.join(", ")}." + end + + lines.join("\n") + end + private def self.layout_line(options : Options) : String orientation = case options.aspect when "9:16" then "Portrait" From 113725c568053d488620f22810509535bc5f7a26 Mon Sep 17 00:00:00 2001 From: Cameron Reeves Date: Fri, 28 Aug 2026 11:30:36 +1000 Subject: [PATCH 07/14] fix(signage-ai): an edit keeps the source's shape An edit came back at the size the aspect dropdown implied, so a 2:3 poster edited as 9:16 was reframed. The aspect describes the screen the artwork is destined for, which is not the shape of the thing handed to us, and an edit must not change the shape at all. The runner already reads the source's dimensions out of the file header, so it now derives the output size from those, snapped to what gpt-image-2 will accept: both edges a multiple of 16, a long edge within 3840, and the total between 655,360 and 8,294,400 pixels. Sources outside the vendor's 3:1 limit fall back to the aspect table. Snapping to 16 moves the area either way and both bounds are hard, so it steps back inside them; a spec caught 8000x6000 landing 12k pixels over the ceiling. Co-Authored-By: Claude Opus 5 (1M context) --- spec/signage_ai_size_spec.cr | 60 +++++++++++++++++ src/placeos-rest-api/utilities/image_gen.cr | 65 +++++++++++++++++++ .../utilities/image_gen/runner.cr | 11 +++- 3 files changed, 135 insertions(+), 1 deletion(-) create mode 100644 spec/signage_ai_size_spec.cr diff --git a/spec/signage_ai_size_spec.cr b/spec/signage_ai_size_spec.cr new file mode 100644 index 00000000..1894bc53 --- /dev/null +++ b/spec/signage_ai_size_spec.cr @@ -0,0 +1,60 @@ +require "./helper" + +module PlaceOS::Api + describe ImageGen do + describe ".editable_size" do + it "keeps the source shape for a poster that is already a legal size" do + # the 2:3 schedule poster, the case that came back reframed to 9:16 + size = ImageGen.editable_size(1080, 1440).not_nil! + width, height = size.split('x').map(&.to_i) + (width / height.to_f).should be_close(1080 / 1440.0, 0.02) + (width % 16).should eq 0 + (height % 16).should eq 0 + end + + it "scales a small source up over the vendor's floor" do + size = ImageGen.editable_size(535, 693).not_nil! + width, height = size.split('x').map(&.to_i) + (width * height).should be >= 655_360 + (width / height.to_f).should be_close(535 / 693.0, 0.02) + end + + it "scales an oversized source down under the ceiling" do + size = ImageGen.editable_size(8000, 6000).not_nil! + width, height = size.split('x').map(&.to_i) + (width * height).should be <= 8_294_400 + width.should be <= 3840 + height.should be <= 3840 + (width / height.to_f).should be_close(8000 / 6000.0, 0.02) + end + + it "gives up on a shape the vendor will not draw" do + # 5:1 banner, past the 3:1 limit: the aspect table takes over + ImageGen.editable_size(5000, 1000).should be_nil + ImageGen.editable_size(1000, 5000).should be_nil + end + + it "refuses nonsense" do + ImageGen.editable_size(0, 100).should be_nil + ImageGen.editable_size(-10, 100).should be_nil + end + + it "always lands on something the vendor accepts" do + { + {1920, 1080}, {1080, 1920}, {1024, 1024}, {2480, 3508}, + {800, 600}, {3000, 1200}, {640, 480}, {4096, 4096}, + }.each do |(width, height)| + size = ImageGen.editable_size(width, height) + next if size.nil? + w, h = size.split('x').map(&.to_i) + (w % 16).should eq 0 + (h % 16).should eq 0 + (w * h).should be >= 655_360 + (w * h).should be <= 8_294_400 + w.should be <= 3840 + h.should be <= 3840 + end + end + end + end +end diff --git a/src/placeos-rest-api/utilities/image_gen.cr b/src/placeos-rest-api/utilities/image_gen.cr index 7d60b49c..b5e7116f 100644 --- a/src/placeos-rest-api/utilities/image_gen.cr +++ b/src/placeos-rest-api/utilities/image_gen.cr @@ -46,6 +46,71 @@ module PlaceOS::Api SIZES[aspect]? || SIZES["16:9"] end + # gpt-image-2 accepts any size with both edges a multiple of 16, a long edge + # no greater than 3840, a ratio within 3:1, and between 655,360 and + # 8,294,400 total pixels. + MIN_PIXELS = 655_360 + MAX_PIXELS = 8_294_400 + MAX_EDGE = 3_840 + MAX_RATIO = 3.0 + + # The size to ask for when editing: the source's own shape, snapped to what + # the vendor accepts. An edit must not reframe the image, and the aspect the + # caller picked describes where the poster will play, not what shape the + # thing they handed us is. + def self.editable_size(width : Int32, height : Int32) : String? + return nil if width <= 0 || height <= 0 + + w = width.to_f + h = height.to_f + + # a shape the vendor will not draw at all: fall back to the aspect table + return nil if (w / h) > MAX_RATIO || (h / w) > MAX_RATIO + + # scale into the pixel budget, then off the longest edge + pixels = w * h + scale = 1.0 + scale = Math.sqrt(MAX_PIXELS / pixels) if pixels > MAX_PIXELS + scale = Math.sqrt(MIN_PIXELS / pixels) if pixels < MIN_PIXELS + w *= scale + h *= scale + + longest = Math.max(w, h) + if longest > MAX_EDGE + edge_scale = MAX_EDGE / longest + w *= edge_scale + h *= edge_scale + end + + # both edges to a multiple of 16, keeping at least the minimum pixels + snapped_w = ((w / 16).round * 16).to_i + snapped_h = ((h / 16).round * 16).to_i + snapped_w = 16 if snapped_w < 16 + snapped_h = 16 if snapped_h < 16 + + # snapping moves the area either way, and both bounds are hard: nudge a + # step at a time until it sits inside them + while snapped_w * snapped_h < MIN_PIXELS + if snapped_w < snapped_h + snapped_w += 16 + else + snapped_h += 16 + end + end + + while snapped_w * snapped_h > MAX_PIXELS && snapped_w > 16 && snapped_h > 16 + if snapped_w > snapped_h + snapped_w -= 16 + else + snapped_h -= 16 + end + end + + return nil if snapped_w > MAX_EDGE || snapped_h > MAX_EDGE + return nil if snapped_w * snapped_h < MIN_PIXELS || snapped_w * snapped_h > MAX_PIXELS + "#{snapped_w}x#{snapped_h}" + end + # shared across the process, sized by SIGNAGE_AI_MAX_CALLS class_getter slots : Slots { Slots.new(SIGNAGE_AI_MAX_CALLS) } end diff --git a/src/placeos-rest-api/utilities/image_gen/runner.cr b/src/placeos-rest-api/utilities/image_gen/runner.cr index 2fc7987b..00a03be5 100644 --- a/src/placeos-rest-api/utilities/image_gen/runner.cr +++ b/src/placeos-rest-api/utilities/image_gen/runner.cr @@ -124,7 +124,16 @@ module PlaceOS::Api::ImageGen if (source_id = context.source_upload_id) upload = ::PlaceOS::Model::Upload.find?(source_id) raise Error::ImageGen::Vendor.new("the source image is gone") if upload.nil? - request = request.copy_with(source: Store.fetch(upload)) + source = Store.fetch(upload) + request = request.copy_with(source: source) + + # an edit comes back at the size we ask for, so ask for the shape we were + # given. Without this the aspect the caller picked reframes the poster. + if (dimensions = Http.dimensions(source.bytes)) + if (size = ImageGen.editable_size(dimensions[0], dimensions[1])) + request = request.copy_with(size_override: size) + end + end end unless context.reference_upload_ids.empty? From 67de6a59253d27e48cf5657afda728bad1e05db3 Mon Sep 17 00:00:00 2001 From: Cameron Reeves Date: Fri, 28 Aug 2026 14:23:54 +1000 Subject: [PATCH 08/14] feat(signage): send attached images to the vendor, and say which is which Three things were wrong with references. They never reached the vendor on a generate. The JSON generations call carries a prompt and nothing else, so anything attached was fetched, counted and dropped. A generate that has references now goes through the multipart endpoint, which is how the vendor takes input images. The model was not told what "image 2" meant. It is handed a list with no names, so a brief naming one was pointing at nothing. The prompt now states how many are attached and in what order, and on an edit says that the first image is the one being edited so the person's first reference is still image 1. The logo was being appended to that same list. It was harmless while generates dropped their images and would not be now: the app composites the real logo afterwards, so an attached one is a logo the model draws as well, leaving two. It goes only on an edit now, which is what already happened in practice. Uploads attached to a request are tagged so the existing sweep can clear them, for the times the browser does not get to. --- spec/signage_ai_prompt_spec.cr | 31 +++++++++++++++++ .../controllers/signage/ai.cr | 20 +++++++++-- .../image_gen/adapters/openai_images.cr | 15 +++++++-- .../utilities/image_gen/prompt.cr | 33 ++++++++++++++++++- 4 files changed, 93 insertions(+), 6 deletions(-) diff --git a/spec/signage_ai_prompt_spec.cr b/spec/signage_ai_prompt_spec.cr index 0c5683cc..b13dc86f 100644 --- a/spec/signage_ai_prompt_spec.cr +++ b/spec/signage_ai_prompt_spec.cr @@ -117,6 +117,37 @@ module PlaceOS::Api end end + it "numbers attached images from one, and says the edited image is not one of them" do + generating = ImageGen::Prompt.build(ImageGen::Prompt::Options.new( + brief: "a poster in the style of image 1 with the person from image 2", + aspect: "16:9", + references: 2, + )) + + generating.should contain "2 image(s) are attached with this request, numbered 1 to 2" + generating.should_not contain "The first attached image is the image being edited" + + editing = ImageGen::Prompt.build(ImageGen::Prompt::Options.new( + brief: "a poster for the office party", + aspect: "16:9", + kind: ImageGen::Kind::Edit, + instruction: "put the person from image 1 on the right", + references: 1, + )) + + editing.should contain "The first attached image is the image being edited." + editing.should contain "The 1 image(s) after it were supplied with this request, numbered 1 to 1" + end + + it "says nothing about attached images when none are attached" do + prompt = ImageGen::Prompt.build(ImageGen::Prompt::Options.new( + brief: "a poster for the office party", + aspect: "16:9", + )) + + prompt.should_not contain "attached" + end + it "carries the brand kit, the brief and each change in order" do prompt = ImageGen::Prompt.build(ImageGen::Prompt::Options.new( brief: "a poster for the office party", diff --git a/src/placeos-rest-api/controllers/signage/ai.cr b/src/placeos-rest-api/controllers/signage/ai.cr index f20d727a..9d208d89 100644 --- a/src/placeos-rest-api/controllers/signage/ai.cr +++ b/src/placeos-rest-api/controllers/signage/ai.cr @@ -496,8 +496,23 @@ module PlaceOS::Api allowed = adapter.capabilities.models.map(&.id) raise Error::ModelValidation.new([Error::Field.new(:model, "is not available on this provider")]) unless allowed.includes?(chosen_model) - reference_ids = references.first(8).map { |id| readable_upload(id).id.as(String) } - if include_logo && (logo = brand_kit.try(&.logo_upload_id).presence) + # Attached by the person, in the order they attached them: the prompt + # numbers them from 1 and the adapter sends them in this order, so the two + # have to agree. + supplied = references.first(8).compact_map { |id| readable_upload(id) } + supplied.each do |upload| + # a bare upload is one made for this request; tagging it lets the sweep + # clear it if the browser never gets the chance to + next unless upload.tags.empty? + upload.tags = [ImageGen::Store::REFERENCE_TAG] + upload.save + end + reference_ids = supplied.map(&.id.as(String)) + + # The logo goes to the vendor only on an edit. On a generate the app + # composites the real file afterwards, and an attached logo is a logo the + # model draws as well, leaving two. + if kind == ImageGen::Kind::Edit && include_logo && (logo = brand_kit.try(&.logo_upload_id).presence) reference_ids << logo unless reference_ids.includes?(logo) end @@ -524,6 +539,7 @@ module PlaceOS::Api words: words, history: history, instruction: editing || parent ? prompt : nil, + references: supplied.size, )) request = ImageGen::AdapterRequest.new( diff --git a/src/placeos-rest-api/utilities/image_gen/adapters/openai_images.cr b/src/placeos-rest-api/utilities/image_gen/adapters/openai_images.cr index 67762b5b..f7337c3b 100644 --- a/src/placeos-rest-api/utilities/image_gen/adapters/openai_images.cr +++ b/src/placeos-rest-api/utilities/image_gen/adapters/openai_images.cr @@ -37,6 +37,11 @@ module PlaceOS::Api::ImageGen::Adapters end def generate(request : AdapterRequest) : Array(AdapterImage) + # Images only travel on the multipart endpoint. The JSON generations call + # takes a prompt and nothing else, so a request carrying references would + # silently lose them. + return with_images(request, nil) unless request.references.empty? + body = { model: request.model, prompt: request.prompt, @@ -53,7 +58,12 @@ module PlaceOS::Api::ImageGen::Adapters def edit(request : AdapterRequest) : Array(AdapterImage) source = request.source raise Error::ImageGen::Vendor.new("an edit needs a source image") if source.nil? + with_images(request, source) + end + # The order here is the order the prompt describes: the image being edited, + # if there is one, then each reference as the person attached it. + private def with_images(request : AdapterRequest, source : Reference?) : Array(AdapterImage) io = IO::Memory.new content_type = "" HTTP::FormData.build(io) do |form| @@ -65,10 +75,9 @@ module PlaceOS::Api::ImageGen::Adapters form.field("quality", quality(request.quality)) form.field("output_format", "jpeg") - # the image being edited goes first, references after it - add_image(form, source, "source") + add_image(form, source, "source") if source request.references.each_with_index do |reference, index| - add_image(form, reference, "reference-#{index}") + add_image(form, reference, "reference-#{index + 1}") end end diff --git a/src/placeos-rest-api/utilities/image_gen/prompt.cr b/src/placeos-rest-api/utilities/image_gen/prompt.cr index 1f6a5d11..a796937e 100644 --- a/src/placeos-rest-api/utilities/image_gen/prompt.cr +++ b/src/placeos-rest-api/utilities/image_gen/prompt.cr @@ -151,7 +151,9 @@ module PlaceOS::Api::ImageGen brand : BrandKit? = nil, words : String? = nil, history : Array(String) = [] of String, - instruction : String? = nil + instruction : String? = nil, + # how many images the person attached, so the brief can name them + references : Int32 = 0 # What an edit is told, in place of the art direction and the layout brief. # @@ -174,6 +176,27 @@ module PlaceOS::Api::ImageGen The result should look like the supplied image with one alteration made to it, not like a new design of the same subject. TEXT + # What "image 2" in a brief means. + # + # The vendor is handed a list of images with no names, so a brief that says + # "the person in image 2" is meaningless unless the order is spelled out. + # On an edit the image being changed is sent first, which would make the + # person's first reference image 2 if that went unsaid. + def self.references_line(count : Int32, editing : Bool) : String? + return nil if count < 1 + lines = [] of String + if editing + lines << "The first attached image is the image being edited." + lines << "The #{count} image(s) after it were supplied with this request, numbered 1 to #{count} in that order." + else + lines << "#{count} image(s) are attached with this request, numbered 1 to #{count} in the order they are sent." + end + lines << %(Where the wording says "image 1", "image 2" and so on, it means those.) + lines << "Use each one as the wording asks: as a guide to style, or as something to include in the picture." + lines << "Do not reproduce an attached image as the whole poster unless asked to, and do not copy any lettering from one." + lines.join(" ") + end + def self.build(options : Options) : String return build_edit(options) if options.kind.edit? @@ -181,6 +204,10 @@ module PlaceOS::Api::ImageGen lines << style(options.text_mode) + if (attached = references_line(options.references, false)) + lines << attached + end + if (brand = options.brand) parts = [] of String parts << "Organisation: #{brand.organisation}." if brand.organisation.presence @@ -213,6 +240,10 @@ module PlaceOS::Api::ImageGen private def self.build_edit(options : Options) : String lines = [EDIT_PRESERVATION] of String + if (attached = references_line(options.references, true)) + lines << attached + end + # what the poster was originally for, so a change is read in context lines << "The image is a #{options.aspect} poster for a digital signage screen." lines << "It was made for this brief: #{options.brief}" if options.brief.presence From 683f426940b243a25acf69a87680f42ca970444a Mon Sep 17 00:00:00 2001 From: Cameron Reeves Date: Fri, 28 Aug 2026 19:39:51 +1000 Subject: [PATCH 09/14] feat(signage): let a request opt out of the organisation's look Not every poster is meant to look like the company. `use_branding` defaults to true and keeps today's behaviour; false leaves the organisation, palette and tone out of the prompt, and is recorded on the job so what a poster was asked for stays readable afterwards. The never-include list is not part of the switch. It says what the organisation will not have on a screen, which holds whether or not the poster is wearing its colours, and turning off "use our colours" is not consent to competitor logos. --- spec/signage_ai_prompt_spec.cr | 14 ++++++++++++++ src/placeos-rest-api/controllers/signage/ai.cr | 9 +++++++++ src/placeos-rest-api/utilities/image_gen/prompt.cr | 9 +++++++-- 3 files changed, 30 insertions(+), 2 deletions(-) diff --git a/spec/signage_ai_prompt_spec.cr b/spec/signage_ai_prompt_spec.cr index b13dc86f..61e10cb7 100644 --- a/spec/signage_ai_prompt_spec.cr +++ b/spec/signage_ai_prompt_spec.cr @@ -148,6 +148,20 @@ module PlaceOS::Api prompt.should_not contain "attached" end + it "leaves the organisation's look out when the branding is switched off" do + plain = ImageGen::Prompt.build(ImageGen::Prompt::Options.new( + brief: "a poster for the office party", + aspect: "16:9", + brand: brand, + use_branding: false, + )) + + plain.should_not contain "Organisation: Acme." + plain.should_not contain "primary #0E6E52" + # a standing prohibition, not a look: it holds either way + plain.should contain "Never include: competitor logos." + end + it "carries the brand kit, the brief and each change in order" do prompt = ImageGen::Prompt.build(ImageGen::Prompt::Options.new( brief: "a poster for the office party", diff --git a/src/placeos-rest-api/controllers/signage/ai.cr b/src/placeos-rest-api/controllers/signage/ai.cr index 9d208d89..26b7f191 100644 --- a/src/placeos-rest-api/controllers/signage/ai.cr +++ b/src/placeos-rest-api/controllers/signage/ai.cr @@ -53,6 +53,8 @@ module PlaceOS::Api getter references : Array(String) = [] of String getter include_logo : Bool = true getter add_text_with_layer : Bool = true + # false leaves the organisation's colours, face and tone out of it + getter use_branding : Bool = true getter words : String? = nil getter provider_id : UUID? = nil getter model : String? = nil @@ -70,6 +72,8 @@ module PlaceOS::Api getter references : Array(String) = [] of String getter include_logo : Bool = true getter add_text_with_layer : Bool = true + # false leaves the organisation's colours, face and tone out of it + getter use_branding : Bool = true getter words : String? = nil getter provider_id : UUID? = nil getter model : String? = nil @@ -197,6 +201,7 @@ module PlaceOS::Api references: params.references, include_logo: params.include_logo, text_layer: params.add_text_with_layer, + use_branding: params.use_branding, words: params.words, provider_id: params.provider_id, model: params.model, @@ -229,6 +234,7 @@ module PlaceOS::Api references: params.references, include_logo: params.include_logo, text_layer: params.add_text_with_layer, + use_branding: params.use_branding, words: params.words, provider_id: params.provider_id, model: params.model, @@ -442,6 +448,7 @@ module PlaceOS::Api references : Array(String), include_logo : Bool, text_layer : Bool, + use_branding : Bool, words : String?, provider_id : UUID?, model : String?, @@ -540,6 +547,7 @@ module PlaceOS::Api history: history, instruction: editing || parent ? prompt : nil, references: supplied.size, + use_branding: use_branding, )) request = ImageGen::AdapterRequest.new( @@ -577,6 +585,7 @@ module PlaceOS::Api "quality" => JSON::Any.new(quality), "include_logo" => JSON::Any.new(include_logo), "text_layer" => JSON::Any.new(text_layer), + "use_branding" => JSON::Any.new(use_branding), "references" => JSON::Any.new(reference_ids.map { |id| JSON::Any.new(id) }), }) job.result = JSON::Any.new({ diff --git a/src/placeos-rest-api/utilities/image_gen/prompt.cr b/src/placeos-rest-api/utilities/image_gen/prompt.cr index a796937e..61e6ea69 100644 --- a/src/placeos-rest-api/utilities/image_gen/prompt.cr +++ b/src/placeos-rest-api/utilities/image_gen/prompt.cr @@ -153,7 +153,9 @@ module PlaceOS::Api::ImageGen history : Array(String) = [] of String, instruction : String? = nil, # how many images the person attached, so the brief can name them - references : Int32 = 0 + references : Int32 = 0, + # off means this poster is not for the organisation's own look + use_branding : Bool = true # What an edit is told, in place of the art direction and the layout brief. # @@ -208,7 +210,10 @@ module PlaceOS::Api::ImageGen lines << attached end - if (brand = options.brand) + # The never-include list is not part of this: it says what the + # organisation will not have on a screen, which holds whether or not the + # poster is wearing its colours. + if (brand = options.brand) && options.use_branding parts = [] of String parts << "Organisation: #{brand.organisation}." if brand.organisation.presence if (colours = brand.palette_line) From c49004a5531362ae423ca9417c5ee9e7dff19749 Mon Sep 17 00:00:00 2001 From: Cameron Reeves Date: Mon, 31 Aug 2026 13:33:27 +1000 Subject: [PATCH 10/14] fix(signage-ai): the blockers and defects found in the pre-push audit Provider rows are scoped to the calling domain. `index` returned every row in the deployment, and every other route took a bare id, so an administrator of one customer could read, change, delete and spend against another's provider. The shared fallback row stays readable everywhere and writable nowhere. A reference upload is only tagged for the sweep when this caller made it in the last fifteen minutes. Tagging any untagged upload marked a file somebody had attached from their library for deletion, dated from the file's own age, so one older than the retention window was eligible immediately. Source and reference images are bounded. The read is streamed and capped rather than buffered and measured afterwards, and an upload is rejected on its recorded size and file type before anything fetches it. The brand logo goes through the size and type check but not the ownership ladder: it is a domain asset nobody personally owns, and putting it through `readable_upload` dropped it from every edit a customer made while still advertising the toggle. `claim` no longer requires the item's file to be the candidate itself. Drawing words over the artwork saves a flattened copy, so the ids never matched and the job to item link was never written for the posters people actually make. Five sites did `version = version + 1` then `save`, which can move the version backwards and strand a long polling client. All of them bump in SQL now. Also: a concurrent duplicate submission replays the job it lost to instead of answering with an empty 422, the Google token call cannot hold a slot forever, the provider test takes a slot like any other vendor call, and the claim scan no longer raises on a job with a candidate that never landed. Specs for the two paths nothing covered: a customer with Create on a signage group generating an image, and the brand logo reaching the vendor for a caller who does not own it. --- spec/controllers/signage/signage_ai_spec.cr | 114 +++++++++++++++++- src/constants.cr | 5 + .../controllers/signage/ai.cr | 85 +++++++++++-- .../controllers/signage/ai_providers.cr | 54 +++++++-- .../image_gen/adapters/gemini_vertex.cr | 26 +++- .../utilities/image_gen/http.cr | 63 +++++++++- .../utilities/image_gen/runner.cr | 6 +- .../utilities/image_gen/sweep.cr | 44 ++++++- 8 files changed, 362 insertions(+), 35 deletions(-) diff --git a/spec/controllers/signage/signage_ai_spec.cr b/spec/controllers/signage/signage_ai_spec.cr index 953b8f0a..aa0d9749 100644 --- a/spec/controllers/signage/signage_ai_spec.cr +++ b/spec/controllers/signage/signage_ai_spec.cr @@ -162,6 +162,78 @@ module PlaceOS::Api await_signage_ai_job(base, job_id, headers) end + # The case a real customer is in. Every other non-support spec here proves + # a refusal, so nothing ever proved the path a paying user actually takes, + # and the browser was not sending group_id at all. + it "lets a non-support caller with Create on a signage group generate" do + authority, _, _ = setup_signage_ai + user, headers = Spec::Authentication.authentication(sys_admin: false, support: false) + group = signage_group(authority, user, Model::Permissions::Read | Model::Permissions::Create) + + HttpMocks.signage_ai_vendor + HttpMocks.signage_ai_storage + + result = client.post( + File.join(base, "generate"), + headers: headers, + body: {prompt: "a poster", candidates: 1, group_id: group.id}.to_json, + ) + + result.status_code.should eq 202 + job_id = JSON.parse(result.body)["id"].as_s + Model::SignageAIJob.find!(UUID.new(job_id)).user_id.should eq user.id + + final = await_signage_ai_job(base, job_id, headers) + final["state"].as_s.should eq "done" + final["images_produced"].as_i.should eq 1 + end + + # The logo is a domain asset nobody personally owns. Routing it through + # the caller-ownership check dropped it from every edit made by a customer + # while still advertising the toggle, so this pins the actual policy. + it "sends the brand logo on an edit for a caller who does not own it" do + authority, storage, _ = setup_signage_ai + owner, _ = Spec::Authentication.authentication + logo = Model::Generator.upload(uploader: owner, storage_id: storage.id) + logo.file_name = "logo.png" + logo.save! + + user, headers = Spec::Authentication.authentication(sys_admin: false, support: false) + + # the org zone the auth helper points every authority at; setting our + # own would be undone by the next authentication call + zone = Spec::Authentication.org_zone + Model::Metadata.where(parent_id: zone.id.as(String), name: "signage_ai").each(&.destroy) + Model::Metadata.new( + parent_id: zone.id.as(String), + name: "signage_ai", + details: JSON.parse({logo_upload_id: logo.id}.to_json), + ).save! + group = signage_group(authority, user, Model::Permissions::Read | Model::Permissions::Create) + source = Model::Generator.upload(uploader: user, storage_id: storage.id) + source.file_name = "source.png" + source.save! + + HttpMocks.signage_ai_vendor + HttpMocks.signage_ai_storage + + result = client.post( + File.join(base, "edit"), + headers: headers, + body: { + prompt: "make it warmer", + candidates: 1, + group_id: group.id, + include_logo: true, + source_upload_id: source.id, + }.to_json, + ) + + result.status_code.should eq 202 + job = Model::SignageAIJob.find!(UUID.new(JSON.parse(result.body)["id"].as_s)) + job.upload_ids.should contain logo.id + end + it "refuses a non-support caller who names no group" do authority, _, _ = setup_signage_ai user, headers = Spec::Authentication.authentication(sys_admin: false, support: false) @@ -424,13 +496,47 @@ module PlaceOS::Api kept.tags.should_not contain ImageGen::Store::CANDIDATE_TAG end - it "refuses an item that does not use the image" do + # The app draws the words over the artwork and saves a flattened copy, so + # the item's file is a new upload derived from the candidate rather than + # the candidate itself. Requiring them to be the same upload meant a claim + # never succeeded for a poster with any words on it. + it "records the item when it is a flattened copy rather than the candidate" do _, storage, provider = setup_signage_ai user, headers = Spec::Authentication.authentication + candidate = Model::Generator.upload(uploader: user, storage_id: storage.id).save! + flattened = Model::Generator.upload(uploader: user, storage_id: storage.id).save! + item = Model::Generator.item(media_id: flattened.id).save! + + job = Model::Generator.signage_ai_job(user: user, provider: provider, candidates: 1) + job.state = Model::SignageAIJob::State::Done + job.result = JSON.parse({images: [{state: "done", index: 0, upload_id: candidate.id}]}.to_json) + job.images_produced = 1 + job.save! + + result = client.post( + File.join(base, "jobs", job.id.to_s, "claim"), + headers: headers, + body: {upload_id: candidate.id, item_id: item.id}.to_json, + ) + + result.status_code.should eq 200 + + stored = Model::SignageAIJob.find!(job.id.as(UUID)) + stored.images.first["item_id"].as_s.should eq item.id + # claiming records, it does not produce: counting again inflated usage + stored.images_produced.should eq 1 + end + + it "refuses an item from another domain" do + _, storage, provider = setup_signage_ai + user, headers = Spec::Authentication.authentication + + other = Model::Generator.authority("other-#{UUID.random}.example.com").save! upload = Model::Generator.upload(uploader: user, storage_id: storage.id).save! - unrelated = Model::Generator.upload(uploader: user, storage_id: storage.id).save! - item = Model::Generator.item(media_id: unrelated.id).save! + item = Model::Generator.item(media_id: upload.id) + item.authority_id = other.id + item.save! job = Model::Generator.signage_ai_job(user: user, provider: provider, candidates: 1) job.state = Model::SignageAIJob::State::Done @@ -443,7 +549,7 @@ module PlaceOS::Api body: {upload_id: upload.id, item_id: item.id}.to_json, ) - result.status_code.should eq 422 + result.status_code.should eq 403 end end diff --git a/src/constants.cr b/src/constants.cr index a928cdff..5db544c3 100644 --- a/src/constants.cr +++ b/src/constants.cr @@ -42,6 +42,11 @@ module PlaceOS::Api SIGNAGE_AI_DISABLED = ENV["SIGNAGE_AI_DISABLED"]?.try(&.downcase) == "true" # default quotas, overridden per provider row + # The largest source or reference image we will pull into the process. The + # whole object is read into memory and the Vertex adapter base64 encodes it, + # so this bounds roughly twice this much per candidate in flight. + SIGNAGE_AI_MAX_IMAGE_BYTES = (ENV["SIGNAGE_AI_MAX_IMAGE_MB"]? || "20").to_i64 * 1024 * 1024 + SIGNAGE_AI_USER_PER_DAY = (ENV["SIGNAGE_AI_USER_PER_DAY"]? || "60").to_i SIGNAGE_AI_DOMAIN_PER_MONTH = (ENV["SIGNAGE_AI_DOMAIN_PER_MONTH"]? || "2000").to_i diff --git a/src/placeos-rest-api/controllers/signage/ai.cr b/src/placeos-rest-api/controllers/signage/ai.cr index 26b7f191..c72a4b12 100644 --- a/src/placeos-rest-api/controllers/signage/ai.cr +++ b/src/placeos-rest-api/controllers/signage/ai.cr @@ -302,8 +302,12 @@ module PlaceOS::Api unless job.final? job.cancel_requested = true - job.version = job.version + 1 job.save + # in SQL, not read-modify-write: a candidate fiber may have moved the + # version since this row was loaded, and writing a stale value back + # leaves a long polling client waiting out its whole timeout + ::PlaceOS::Model::SignageAIJob.bump_version(job.id.as(UUID)) + job = ::PlaceOS::Model::SignageAIJob.find!(job.id.as(UUID)) end JobResponse.new(job) @@ -317,9 +321,19 @@ module PlaceOS::Api item = ::PlaceOS::Model::Playlist::Item.find!(params.item_id) raise Error::Forbidden.new("item belongs to another domain") unless item.authority_id == authority.id - raise Error::ModelValidation.new([Error::Field.new(:item_id, "does not use this image")]) unless item.media_id == params.upload_id - index = job.images.index { |image| image["upload_id"]?.try(&.as_s?) == params.upload_id } + # The item's file does not have to BE the candidate. When the app draws + # the words and the logo over the artwork it saves a flattened copy, so + # the item points at a new upload derived from the candidate. Requiring + # the two to match meant a claim never succeeded for any poster with + # words on it, which is most of them, and the provenance link was silently + # lost. Owning the job and the item is the check that matters. + + # a slot for a candidate that never landed is a JSON null, so `[]?` on it + # raises rather than answering nil + index = job.images.index do |image| + image.as_h?.try(&.["upload_id"]?).try(&.as_s?) == params.upload_id + end raise Error::NotFound.new("that image is not part of this job") if index.nil? upload = ::PlaceOS::Model::Upload.find?(params.upload_id) @@ -332,7 +346,8 @@ module PlaceOS::Api entry = job.images[index].as_h entry["item_id"] = JSON::Any.new(item.id.as(String)) - ::PlaceOS::Model::SignageAIJob.bump_image(job.id.as(UUID), index, entry) + # deliberately not bump_image: the runner already counted this candidate + ::PlaceOS::Model::SignageAIJob.attach_item(job.id.as(UUID), index, entry) JobResponse.new(::PlaceOS::Model::SignageAIJob.find!(job.id.as(UUID))) end @@ -398,10 +413,36 @@ module PlaceOS::Api # An upload the caller may use as a source or a reference: one they own, or # one behind a media item in this domain they can read. Uploads carry no # authority of their own, so an item is how a shared image is proved. + # How recently an upload must have been made for us to treat it as a + # throwaway attached to this request rather than a file from the library. + REFERENCE_TAG_WINDOW = 15.minutes + + # File types the vendors take, and that `Http.mime_of` can identify from the + # bytes. Checked on the name because an upload records no content type. + IMAGE_EXTENSIONS = {".png", ".jpg", ".jpeg", ".webp"} + + # Size and type only, with no view on who owns it. Separate from + # `readable_upload` because the brand logo is a domain asset that belongs to + # nobody in particular: putting it through the ownership ladder below + # dropped it for every caller who was not support, which is every customer. + private def vendor_readable?(upload : ::PlaceOS::Model::Upload) : Bool + return false if upload.file_size > SIGNAGE_AI_MAX_IMAGE_BYTES + IMAGE_EXTENSIONS.includes?(File.extname(upload.file_name).downcase) + end + private def readable_upload(upload_id : String, item_id : String? = nil) : ::PlaceOS::Model::Upload upload = ::PlaceOS::Model::Upload.find?(upload_id) raise Error::NotFound.new("no such upload") if upload.nil? + # Bound this before anything reads the object: `Store.fetch` pulls the + # whole thing into the heap and the Vertex adapter base64 encodes it. + if upload.file_size > SIGNAGE_AI_MAX_IMAGE_BYTES + raise Error::ImageGen::Permission.new("that image is larger than #{SIGNAGE_AI_MAX_IMAGE_BYTES // (1024 * 1024)}MB") + end + unless IMAGE_EXTENSIONS.includes?(File.extname(upload.file_name).downcase) + raise Error::ImageGen::Permission.new("that file is not a png, jpeg or webp image") + end + return upload if upload.uploaded_by == current_user.id return upload if user_support? @@ -508,9 +549,13 @@ module PlaceOS::Api # have to agree. supplied = references.first(8).compact_map { |id| readable_upload(id) } supplied.each do |upload| - # a bare upload is one made for this request; tagging it lets the sweep - # clear it if the browser never gets the chance to + # Only an upload this person just made, for this request: the tag marks + # it for deletion by the sweep, and the sweep counts from the upload's + # own age. Tagging anything older was marking a file the person had + # attached from their library for immediate deletion. next unless upload.tags.empty? + next unless upload.uploaded_by == current_user.id + next unless upload.created_at > REFERENCE_TAG_WINDOW.ago upload.tags = [ImageGen::Store::REFERENCE_TAG] upload.save end @@ -520,7 +565,20 @@ module PlaceOS::Api # composites the real file afterwards, and an attached logo is a logo the # model draws as well, leaving two. if kind == ImageGen::Kind::Edit && include_logo && (logo = brand_kit.try(&.logo_upload_id).presence) - reference_ids << logo unless reference_ids.includes?(logo) + # Size and type, not ownership: it is fetched and base64 encoded exactly + # like a reference, so it needs the same ceiling, but nobody "owns" it. + # An SVG logo is skipped here on purpose: it draws fine in the browser + # layer, and no vendor takes one as an input image. + logo_upload = ::PlaceOS::Model::Upload.find?(logo) + if logo_upload && vendor_readable?(logo_upload) + reference_ids << logo unless reference_ids.includes?(logo) + else + Log.info { { + message: "signage AI brand logo not sent to the vendor", + upload: logo, + reason: logo_upload.nil? ? "missing" : "size or file type", + } } + end end chain = parent ? (parent.chain.compact_map(&.prompt) + [parent.prompt].compact) : [] of String @@ -595,6 +653,19 @@ module PlaceOS::Api unless job.save ImageGen.slots.release(calls) + + # A concurrent submission of the same key got there first. The check + # above is a read followed by an insert with nothing serialising them, + # so the partial unique index is what actually enforces this, and + # answering with the job that won is the point of the key. Without this + # a double submit came back as a validation error rather than a replay. + if (key = idempotency_key.presence) + existing = on_primary do + ::PlaceOS::Model::SignageAIJob.where(user_id: current_user.id.as(String), idempotency_key: key).first? + end + return JobResponse.new(existing) if existing + end + raise Error::ModelValidation.new(job.errors) end diff --git a/src/placeos-rest-api/controllers/signage/ai_providers.cr b/src/placeos-rest-api/controllers/signage/ai_providers.cr index 29fc0bd6..a99ec18f 100644 --- a/src/placeos-rest-api/controllers/signage/ai_providers.cr +++ b/src/placeos-rest-api/controllers/signage/ai_providers.cr @@ -24,7 +24,22 @@ module PlaceOS::Api @[AC::Route::Filter(:before_action, except: [:index, :create])] def find_current_provider(id : UUID) Log.context.set(signage_ai_provider: id.to_s) - @current_provider = ::PlaceOS::Model::SignageAIProvider.find!(id) + row = ::PlaceOS::Model::SignageAIProvider.find!(id) + + # A row belongs to one domain, and the admin flag on a JWT is per domain, + # so without this an administrator of one customer could read, change, + # delete and spend against another customer's provider by guessing an id. + # The shared fallback row (no authority) is readable by everyone and + # writable by nobody through this route. + unless row.authority_id == current_authority.try(&.id) + raise Error::NotFound.new("no such provider") unless row.authority_id.nil? && read_only_action? + end + + @current_provider = row + end + + private def read_only_action? : Bool + request.method.upcase == "GET" end getter! current_provider : ::PlaceOS::Model::SignageAIProvider @@ -63,22 +78,30 @@ module PlaceOS::Api created_at: Int64, updated_at: Int64) - # rows for a domain, or every row when no domain is given + # The rows this domain may use: its own, and the shared fallback. + # + # There is deliberately no way to list another domain's rows. The previous + # version returned every row in the deployment when called without a + # parameter, which handed one customer's endpoint, model list and quotas to + # any administrator of any other. @[AC::Route::GET("/")] def index( - @[AC::Param::Info(description: "return the rows belonging to this authority", example: "authority-1234")] - authority_id : String? = nil, @[AC::Param::Info(description: "include the shared fallback row", example: "true")] include_shared : Bool = true, ) : Array(ProviderJSON) - rows = if authority_id + domain = current_authority.try(&.id) + rows = if domain.nil? + [] of ::PlaceOS::Model::SignageAIProvider + else + # `available_for` is what a generate uses, so it filters to + # enabled rows. This is the admin list: a row somebody switched + # off still has to be visible, or it can never be switched back on. + own = ::PlaceOS::Model::SignageAIProvider.where(authority_id: domain).to_a if include_shared - ::PlaceOS::Model::SignageAIProvider.available_for(authority_id) + own + ::PlaceOS::Model::SignageAIProvider.where(authority_id: nil).to_a else - ::PlaceOS::Model::SignageAIProvider.where(authority_id: authority_id).to_a + own end - else - ::PlaceOS::Model::SignageAIProvider.all.to_a end rows.map { |row| row.as_json } @@ -105,7 +128,9 @@ module PlaceOS::Api end row.credentials = credentials.to_json - row.authority_id = params.authority_id + # the caller's own domain, whatever the body says: a domain administrator + # cannot create a row that belongs to somebody else + row.authority_id = current_authority.try(&.id) apply_optional(row, params) raise Error::ModelValidation.new(row.errors) unless row.save @@ -152,6 +177,13 @@ module PlaceOS::Api adapter = ImageGen::Adapter.for(row) model = row.default_model || adapter.capabilities.default_model + # Take a slot like any other vendor call. Without it a run of clicks on + # this button could occupy every worker and starve real generations, and + # the button is one click with no confirmation. + unless ImageGen.slots.try_reserve(1) + raise Error::ImageGen::Busy.new("too many images are being generated right now, try again in a moment") + end + started = Time.utc begin raise Error::ImageGen::NotConfigured.new("no model configured") if model.nil? @@ -173,6 +205,8 @@ module PlaceOS::Api TestResult.new(false, (Time.utc - started).total_milliseconds.to_i64, model, ex.message, ex.kind) rescue ex TestResult.new(false, (Time.utc - started).total_milliseconds.to_i64, model, ex.message, "vendor") + ensure + ImageGen.slots.release(1) end end diff --git a/src/placeos-rest-api/utilities/image_gen/adapters/gemini_vertex.cr b/src/placeos-rest-api/utilities/image_gen/adapters/gemini_vertex.cr index 4fbe90f7..9871830e 100644 --- a/src/placeos-rest-api/utilities/image_gen/adapters/gemini_vertex.cr +++ b/src/placeos-rest-api/utilities/image_gen/adapters/gemini_vertex.cr @@ -136,10 +136,34 @@ module PlaceOS::Api::ImageGen::Adapters } end + # How long to wait for Google to hand back an access token. + TOKEN_TIMEOUT = 20.seconds + + # The token call does not go through `Http.client`: it is made by the google + # shard, which builds its own client with no timeout. It runs inside the + # candidate fiber, holding a slot, so a hung call held that slot for the + # life of the process. Bounding the wait here releases the slot even if the + # request behind it never comes back. private def token : String issuer = credential("client_email") key = credential("private_key") - Auth.new(issuer: issuer, signing_key: key, scopes: SCOPE).get_token.access_token + + channel = Channel(String | Exception).new(1) + spawn(name: "vertex-token") do + begin + channel.send(Auth.new(issuer: issuer, signing_key: key, scopes: SCOPE).get_token.access_token) + rescue ex + channel.send(ex) + end + end + + select + when result = channel.receive + raise result if result.is_a?(Exception) + result + when timeout(TOKEN_TIMEOUT) + raise Error::ImageGen::Vendor.new("timed out authenticating with Google") + end rescue ex : Error::ImageGen raise ex rescue ex diff --git a/src/placeos-rest-api/utilities/image_gen/http.cr b/src/placeos-rest-api/utilities/image_gen/http.cr index b037f9b7..54ae213a 100644 --- a/src/placeos-rest-api/utilities/image_gen/http.cr +++ b/src/placeos-rest-api/utilities/image_gen/http.cr @@ -15,6 +15,16 @@ module PlaceOS::Api::ImageGen client.connect_timeout = CONNECT_TIMEOUT client.read_timeout = read_timeout client.write_timeout = read_timeout + + # Behind a proxy the socket is opened during construction, before the + # lines above run, and HTTP::Client only applies its timeouts to sockets + # it opens itself. Without this the timeouts silently do not hold on + # exactly the deployment the proxy shard exists for, and a stalled vendor + # keeps its fiber and its slot for good. + if (io = client.@io).is_a?(::Socket) + io.read_timeout = read_timeout + io.write_timeout = read_timeout + end begin yield client ensure @@ -25,20 +35,61 @@ module PlaceOS::Api::ImageGen # Fetch bytes we already hold a signed URL for. Crystal's HTTP::Client does # not follow redirects, so this follows a small number by hand: `/uploads/:id/url` # answers with a 303 to the storage provider. + private def self.too_large : NoReturn + raise Error::ImageGen::Vendor.new("image is larger than #{SIGNAGE_AI_MAX_IMAGE_BYTES // (1024 * 1024)}MB") + end + + # Copy at most `limit` bytes, then give up. Reading the body first and + # checking its size afterwards is not a limit: the allocation has already + # happened, and an upload's recorded size is client supplied, so this is the + # only place the ceiling can actually be enforced. + private def self.read_capped(io : IO, limit : Int64) : Bytes + buffer = IO::Memory.new + copied = IO.copy(io, buffer, limit + 1) + too_large if copied > limit + buffer.to_slice + end + def self.get_bytes(url : String, limit : Int32 = 3) : Tuple(Bytes, String) current = url limit.times do uri = URI.parse(current) - response = client(uri, 60.seconds) { |http| http.get(uri.request_target) } + redirect = nil.as(String?) + result = nil.as(Tuple(Bytes, String)?) + + client(uri, 60.seconds) do |http| + # Streamed, so an oversized object is abandoned rather than buffered. + # Encoding is left alone: asking for identity would turn off Crystal's + # implicit decompression, and a server that ignored it would hand back + # gzip bytes to be stored as an image. The cap below reads the + # decompressed stream, so it holds either way. + http.get(uri.request_target) do |response| + if response.status.redirection? && (location = response.headers["Location"]?) + redirect = location.starts_with?("http") ? location : uri.resolve(location).to_s + next + end + + raise Error::ImageGen::Vendor.new("could not read image (#{response.status_code})") unless response.success? + + # a length we can trust saves reading anything at all + if (length = response.headers["Content-Length"]?.try(&.to_i64?)) && length > SIGNAGE_AI_MAX_IMAGE_BYTES + too_large + end + + body = response.body_io? + raise Error::ImageGen::Vendor.new("image response had no body") if body.nil? + bytes = read_capped(body, SIGNAGE_AI_MAX_IMAGE_BYTES) + mime = response.headers["Content-Type"]? || "application/octet-stream" + result = {bytes, mime.split(';').first.strip} + end + end - if response.status.redirection? && (location = response.headers["Location"]?) - current = location.starts_with?("http") ? location : URI.parse(current).resolve(location).to_s + if (location = redirect) + current = location next end - raise Error::ImageGen::Vendor.new("could not read image (#{response.status_code})") unless response.success? - mime = response.headers["Content-Type"]? || "application/octet-stream" - return {response.body.to_slice, mime.split(';').first.strip} + return result.not_nil! end raise Error::ImageGen::Vendor.new("too many redirects reading image") end diff --git a/src/placeos-rest-api/utilities/image_gen/runner.cr b/src/placeos-rest-api/utilities/image_gen/runner.cr index 00a03be5..c922c46d 100644 --- a/src/placeos-rest-api/utilities/image_gen/runner.cr +++ b/src/placeos-rest-api/utilities/image_gen/runner.cr @@ -45,8 +45,8 @@ module PlaceOS::Api::ImageGen job.state = ::PlaceOS::Model::SignageAIJob::State::Running job.started_at = started - job.version = job.version + 1 job.save + ::PlaceOS::Model::SignageAIJob.bump_version(job.id.as(UUID)) request = hydrate(context) @@ -174,7 +174,6 @@ module PlaceOS::Api::ImageGen job.finished_at = Time.utc job.latency_ms = (Time.utc - started).total_milliseconds.to_i64 job.cost_units = cost if cost > 0 - job.version = job.version + 1 if job.cancel_requested job.state = ::PlaceOS::Model::SignageAIJob::State::Cancelled @@ -196,6 +195,7 @@ module PlaceOS::Api::ImageGen end job.save + ::PlaceOS::Model::SignageAIJob.bump_version(job.id.as(UUID)) Log.info { { message: "signage AI job finished", @@ -214,8 +214,8 @@ module PlaceOS::Api::ImageGen job.error_kind = kind_of(error) job.error_message = message_of(error) job.finished_at = Time.utc - job.version = job.version + 1 job.save + ::PlaceOS::Model::SignageAIJob.bump_version(job.id.as(UUID)) end private def self.kind_of(error : Exception) : String diff --git a/src/placeos-rest-api/utilities/image_gen/sweep.cr b/src/placeos-rest-api/utilities/image_gen/sweep.cr index f0628a4c..0882aea6 100644 --- a/src/placeos-rest-api/utilities/image_gen/sweep.cr +++ b/src/placeos-rest-api/utilities/image_gen/sweep.cr @@ -1,5 +1,6 @@ require "placeos-driver/storage" require "placeos-models/playlist/item" +require "placeos-models/metadata" require "placeos-models/signage_ai_job" require "placeos-models/upload" @@ -60,8 +61,8 @@ module PlaceOS::Api::ImageGen job.error_kind = "timeout" job.error_message = "the job did not finish" job.finished_at = Time.utc - job.version = job.version + 1 job.save + ::PlaceOS::Model::SignageAIJob.bump_version(job.id.as(UUID)) Log.warn { {message: "signage AI job expired", job: job.id.to_s} } end end @@ -80,9 +81,12 @@ module PlaceOS::Api::ImageGen return if uploads.empty? + # read once, not once per upload: this is every tenant's brand kit + logos = brand_logo_ids + uploads.each do |upload| id = upload.id.as(String) - next if referenced?(id) + next if referenced?(id, logos) begin if (storage = upload.storage) @@ -96,11 +100,43 @@ module PlaceOS::Api::ImageGen end end - # kept if a media item points at it, either as the artwork or its thumbnail - private def self.referenced?(upload_id : String) : Bool + # kept if a media item points at it, either as the artwork or its thumbnail, + # or if a brand kit does + private def self.referenced?(upload_id : String, logos : Set(String)?) : Bool + # nil means the brand kits could not be read, and a read that failed is + # not permission to delete + return true if logos.nil? + return true if logos.includes?(upload_id) + ::PlaceOS::Model::Playlist::Item .where("media_id = ? OR thumbnail_id = ?", upload_id, upload_id) .count > 0 end + + # Every logo any brand kit points at. + # + # A logo is uploaded through the same untagged path a throwaway reference + # uses, and is pointed at only from zone metadata, which nothing else here + # looks at. Without this a logo that had been attached to a request as a + # reference would be swept and the brand kit left pointing at a dead file. + # + # nil rather than an empty set when the read fails, so a database fault + # cannot read as "no brand kit has any logos". + private def self.brand_logo_ids : Set(String)? + ids = Set(String).new + ::PlaceOS::Model::Metadata.where(name: "signage_ai").each do |metadata| + details = metadata.details.as_h? + next unless details + {"logo_upload_id", "logo_dark_upload_id"}.each do |key| + if (id = details[key]?.try(&.as_s?)) + ids << id + end + end + end + ids + rescue ex + Log.warn(exception: ex) { "could not read brand kits, keeping every upload this pass" } + nil + end end end From 0180d4ddcae122334af229d4a54f2d2c2a4a07fa Mon Sep 17 00:00:00 2001 From: Cameron Reeves Date: Mon, 31 Aug 2026 14:13:00 +1000 Subject: [PATCH 11/14] fix(signage-ai): bound a whole request, and pin the domain scoping in specs The per-image ceiling still allowed eight attachments at once, so one request could pull 160MB into the process. Capped in aggregate. Three specs for the headline blocker of the last commit, which had none: another domain's provider is absent from the list and 404s on show, update, delete and test; the shared row reads but does not write; and a create lands in the caller's domain whatever the body asks for. --- .../signage/signage_ai_providers_spec.cr | 61 +++++++++++++++++++ src/constants.cr | 4 ++ .../controllers/signage/ai.cr | 9 +++ 3 files changed, 74 insertions(+) diff --git a/spec/controllers/signage/signage_ai_providers_spec.cr b/spec/controllers/signage/signage_ai_providers_spec.cr index 474f5591..83b6aa17 100644 --- a/spec/controllers/signage/signage_ai_providers_spec.cr +++ b/spec/controllers/signage/signage_ai_providers_spec.cr @@ -92,6 +92,67 @@ module PlaceOS::Api Model::SignageAIProvider.find?(provider.id.as(UUID)).should_not be_nil end + # The headline of the fix round: a domain administrator could read, change, + # delete and spend against another customer's provider by guessing an id. + describe "domain scoping" do + it "hides another domain's rows from the list and from every route" do + authority, _, _ = setup_signage_ai + headers = Spec::Authentication.headers(sys_admin: true, support: true) + + other = Model::Generator.authority("other-#{UUID.random}.example.com").save! + theirs = Model::Generator.signage_ai_provider( + authority: other, + name: "theirs-#{random_name}", + ).save! + + listed = JSON.parse(client.get(base, headers: headers).body).as_a + listed.map(&.["id"].as_s).should_not contain theirs.id.to_s + + path = File.join(base, theirs.id.to_s) + client.get(path, headers: headers).status_code.should eq 404 + client.patch(path, headers: headers, body: {name: "mine now"}.to_json).status_code.should eq 404 + client.delete(path, headers: headers).status_code.should eq 404 + client.post(File.join(path, "test"), headers: headers).status_code.should eq 404 + + Model::SignageAIProvider.find!(theirs.id.as(UUID)).name.should eq theirs.name + end + + it "lets a domain read the shared row but not change it" do + setup_signage_ai + headers = Spec::Authentication.headers(sys_admin: true, support: true) + + shared = Model::Generator.signage_ai_provider(name: "shared-#{random_name}") + shared.authority_id = nil + shared.save! + + path = File.join(base, shared.id.to_s) + client.get(path, headers: headers).status_code.should eq 200 + client.patch(path, headers: headers, body: {name: "changed"}.to_json).status_code.should eq 404 + client.delete(path, headers: headers).status_code.should eq 404 + end + + it "puts a new row in the caller's domain whatever the body says" do + authority, _, _ = setup_signage_ai + headers = Spec::Authentication.headers(sys_admin: true, support: true) + + other = Model::Generator.authority("other-#{UUID.random}.example.com").save! + + result = client.post( + base, + headers: headers, + body: { + name: "planted-#{random_name}", + provider: "OPENAI", + authority_id: other.id, + credentials: {api_key: "sk-not-a-real-key"}, + }.to_json, + ) + + result.status_code.should eq 201 + JSON.parse(result.body)["authority_id"].as_s.should eq authority.id + end + end + it "keeps a regular user out entirely" do authority = Model::Authority.find_by_domain("localhost").not_nil! provider = Model::Generator.signage_ai_provider( diff --git a/src/constants.cr b/src/constants.cr index 5db544c3..18728e7e 100644 --- a/src/constants.cr +++ b/src/constants.cr @@ -47,6 +47,10 @@ module PlaceOS::Api # so this bounds roughly twice this much per candidate in flight. SIGNAGE_AI_MAX_IMAGE_BYTES = (ENV["SIGNAGE_AI_MAX_IMAGE_MB"]? || "20").to_i64 * 1024 * 1024 + # And what every attachment on one request may come to together: the + # per-image ceiling alone still allows eight of them at once. + SIGNAGE_AI_MAX_REQUEST_BYTES = (ENV["SIGNAGE_AI_MAX_REQUEST_MB"]? || "48").to_i64 * 1024 * 1024 + SIGNAGE_AI_USER_PER_DAY = (ENV["SIGNAGE_AI_USER_PER_DAY"]? || "60").to_i SIGNAGE_AI_DOMAIN_PER_MONTH = (ENV["SIGNAGE_AI_DOMAIN_PER_MONTH"]? || "2000").to_i diff --git a/src/placeos-rest-api/controllers/signage/ai.cr b/src/placeos-rest-api/controllers/signage/ai.cr index c72a4b12..c7241b05 100644 --- a/src/placeos-rest-api/controllers/signage/ai.cr +++ b/src/placeos-rest-api/controllers/signage/ai.cr @@ -548,6 +548,15 @@ module PlaceOS::Api # numbers them from 1 and the adapter sends them in this order, so the two # have to agree. supplied = references.first(8).compact_map { |id| readable_upload(id) } + + # Eight references each under the per-image ceiling is still 160MB pulled + # into one request. The per-object check does not bound the request. + total = supplied.sum(&.file_size) + if total > SIGNAGE_AI_MAX_REQUEST_BYTES + raise Error::ImageGen::Permission.new( + "those images come to more than #{SIGNAGE_AI_MAX_REQUEST_BYTES // (1024 * 1024)}MB together" + ) + end supplied.each do |upload| # Only an upload this person just made, for this request: the tag marks # it for deletion by the sweep, and the sweep counts from the upload's From 1e8c84120e5c5712d75f1873d254db8c5ec17024 Mon Sep 17 00:00:00 2001 From: Cameron Reeves Date: Mon, 31 Aug 2026 14:48:40 +1000 Subject: [PATCH 12/14] style(signage-ai): crystal tool format CI runs crystal-style on every push and the spec file was not formatted. --- spec/controllers/signage/signage_ai_spec.cr | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/spec/controllers/signage/signage_ai_spec.cr b/spec/controllers/signage/signage_ai_spec.cr index aa0d9749..69a492c2 100644 --- a/spec/controllers/signage/signage_ai_spec.cr +++ b/spec/controllers/signage/signage_ai_spec.cr @@ -221,11 +221,11 @@ module PlaceOS::Api File.join(base, "edit"), headers: headers, body: { - prompt: "make it warmer", - candidates: 1, - group_id: group.id, - include_logo: true, - source_upload_id: source.id, + prompt: "make it warmer", + candidates: 1, + group_id: group.id, + include_logo: true, + source_upload_id: source.id, }.to_json, ) From c373e0e0f840869ade5cf2c4dd991ab8b4875738 Mon Sep 17 00:00:00 2001 From: Cameron Reeves Date: Tue, 1 Sep 2026 08:26:12 +1000 Subject: [PATCH 13/14] build(deps): pin placeos-models 9.109.0 The signage AI models landed in models#326 and the bump automation released them as 9.109.0. CI last built this branch four hours before that tag existed, so it resolved 9.108.2 and failed to compile with `while requiring "./config"`. --- shard.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shard.lock b/shard.lock index 2511eea6..ffa21179 100644 --- a/shard.lock +++ b/shard.lock @@ -223,7 +223,7 @@ shards: placeos-models: git: https://github.com/placeos/models.git - version: 9.108.2 + version: 9.109.0 placeos-resource: git: https://github.com/place-labs/resource.git From 807eb9cf9993d94bf190aeef3620f032031c7e73 Mon Sep 17 00:00:00 2001 From: Cameron Reeves Date: Tue, 1 Sep 2026 08:46:54 +1000 Subject: [PATCH 14/14] style(signage-ai): clear the 15 Ameba issues in the new code CI runs Ameba with todo_issues, so existing findings are grandfathered and only new code is judged. Everything here was mine. Boolean getters become `getter?`, which is the repo's own convention once the linter is looking. The ivar name is unchanged, so JSON::Serializable is unaffected; only the call sites move to the `?` form. Two specs used `index {...}.not_nil!` and `find {...}.not_nil!` where the bang variants say the same thing without the intermediate nil, and one map grew the short block form. --- spec/controllers/signage/signage_ai_spec.cr | 2 +- spec/signage_ai_prompt_spec.cr | 2 +- .../controllers/signage/ai.cr | 28 +++++++++---------- .../controllers/signage/ai_providers.cr | 2 +- .../utilities/image_gen/types.cr | 6 ++-- 5 files changed, 20 insertions(+), 20 deletions(-) diff --git a/spec/controllers/signage/signage_ai_spec.cr b/spec/controllers/signage/signage_ai_spec.cr index 69a492c2..09724d63 100644 --- a/spec/controllers/signage/signage_ai_spec.cr +++ b/spec/controllers/signage/signage_ai_spec.cr @@ -568,7 +568,7 @@ module PlaceOS::Api result.status_code.should eq 200 rows = Array(Model::SignageAIJob::UsageRow).from_json(result.body) - row = rows.find { |entry| entry.provider == "OPENAI" }.not_nil! + row = rows.find! { |entry| entry.provider == "OPENAI" } row.model.should eq "gpt-image-2" row.jobs.should eq 1 row.candidates.should eq 2 diff --git a/spec/signage_ai_prompt_spec.cr b/spec/signage_ai_prompt_spec.cr index 61e10cb7..d5fdf8d2 100644 --- a/spec/signage_ai_prompt_spec.cr +++ b/spec/signage_ai_prompt_spec.cr @@ -20,7 +20,7 @@ module PlaceOS::Api prompt.should contain "AVOID:" prompt.should contain "purple-blue-orange gradients" # the style has to land before the brief, not after it - prompt.index("Avoid the generic").not_nil!.should be < prompt.index("Brief:").not_nil! + prompt.index!("Avoid the generic").should be < prompt.index!("Brief:") end it "holds back the typography lines when the app is setting the type" do diff --git a/src/placeos-rest-api/controllers/signage/ai.cr b/src/placeos-rest-api/controllers/signage/ai.cr index c7241b05..e9ec17cc 100644 --- a/src/placeos-rest-api/controllers/signage/ai.cr +++ b/src/placeos-rest-api/controllers/signage/ai.cr @@ -51,10 +51,10 @@ module PlaceOS::Api getter quality : String = "standard" getter candidates : Int32 = 2 getter references : Array(String) = [] of String - getter include_logo : Bool = true - getter add_text_with_layer : Bool = true + getter? include_logo : Bool = true + getter? add_text_with_layer : Bool = true # false leaves the organisation's colours, face and tone out of it - getter use_branding : Bool = true + getter? use_branding : Bool = true getter words : String? = nil getter provider_id : UUID? = nil getter model : String? = nil @@ -70,10 +70,10 @@ module PlaceOS::Api getter quality : String = "standard" getter candidates : Int32 = 1 getter references : Array(String) = [] of String - getter include_logo : Bool = true - getter add_text_with_layer : Bool = true + getter? include_logo : Bool = true + getter? add_text_with_layer : Bool = true # false leaves the organisation's colours, face and tone out of it - getter use_branding : Bool = true + getter? use_branding : Bool = true getter words : String? = nil getter provider_id : UUID? = nil getter model : String? = nil @@ -98,14 +98,14 @@ module PlaceOS::Api struct Capabilities include JSON::Serializable - getter enabled : Bool + getter? enabled : Bool getter reason : String? getter providers : Array(ImageGen::ProviderCapabilities) getter default_provider_id : String? getter aspect_ratios : Array(String) getter qualities : Array(String) getter max_candidates : Int32 - getter logo_layer : Bool + getter? logo_layer : Bool getter quota : NamedTuple(user_remaining_today: Int32?, domain_remaining_month: Int32?) def initialize(@enabled, @providers, @default_provider_id, @quota, @@ -199,9 +199,9 @@ module PlaceOS::Api quality: params.quality, candidates: params.candidates, references: params.references, - include_logo: params.include_logo, - text_layer: params.add_text_with_layer, - use_branding: params.use_branding, + include_logo: params.include_logo?, + text_layer: params.add_text_with_layer?, + use_branding: params.use_branding?, words: params.words, provider_id: params.provider_id, model: params.model, @@ -232,9 +232,9 @@ module PlaceOS::Api quality: params.quality, candidates: params.candidates, references: params.references, - include_logo: params.include_logo, - text_layer: params.add_text_with_layer, - use_branding: params.use_branding, + include_logo: params.include_logo?, + text_layer: params.add_text_with_layer?, + use_branding: params.use_branding?, words: params.words, provider_id: params.provider_id, model: params.model, diff --git a/src/placeos-rest-api/controllers/signage/ai_providers.cr b/src/placeos-rest-api/controllers/signage/ai_providers.cr index a99ec18f..5aa362ef 100644 --- a/src/placeos-rest-api/controllers/signage/ai_providers.cr +++ b/src/placeos-rest-api/controllers/signage/ai_providers.cr @@ -104,7 +104,7 @@ module PlaceOS::Api end end - rows.map { |row| row.as_json } + rows.map(&.as_json) end @[AC::Route::GET("/:id")] diff --git a/src/placeos-rest-api/utilities/image_gen/types.cr b/src/placeos-rest-api/utilities/image_gen/types.cr index da4bfafa..7a675e09 100644 --- a/src/placeos-rest-api/utilities/image_gen/types.cr +++ b/src/placeos-rest-api/utilities/image_gen/types.cr @@ -52,9 +52,9 @@ module PlaceOS::Api::ImageGen getter id : String getter name : String - getter generate : Bool - getter edit : Bool - getter enhance : Bool + getter? generate : Bool + getter? edit : Bool + getter? enhance : Bool getter max_references : Int32 getter max_candidates : Int32 getter qualities : Array(String)