fix(map-ai): review fixes and shared AI plumbing for the map editing backend

spec/llm_anthropic_spec.cr

@@ -23,6 +23,25 @@ module PlaceOS::Api
       end
     end
 
+    it "reports a proposal cut off at max_tokens as length so the runner can retry" do
+      row = Model::Generator.signage_ai_provider(provider: Model::SignageAIProvider::Provider::ANTHROPIC)
+      row.credentials_json = {"api_key" => JSON::Any.new("test-key")}
+      adapter = Llm::Adapters::AnthropicMessages.new(row)
+      request = Llm::Request.new(system: "x", parts: [Llm::TextPart.new("x")] of Llm::Part,
+        model: "claude-fable-5-1", max_output_tokens: 100, schema: Llm::Schema.openai)
+      [
+        %([{"type":"text","text":"Working through the labels first"}]),
+        %([{"type":"tool_use","name":"map_edit","input":{"schema_version":1,"summary":"Renames"}}]),
+      ].each do |blocks|
+        WebMock.reset
+        WebMock.stub(:post, HttpMocks::ANTHROPIC_MESSAGES).to_return(
+          status: 200, body: %({"stop_reason":"max_tokens","content":#{blocks}}))
+        result = adapter.complete(request)
+        result.finish.should eq "length"
+        result.text.should eq ""
+      end
+    end
+
     it "reads the proposal tool input and ignores accompanying prose" do
       row = Model::Generator.signage_ai_provider(provider: Model::SignageAIProvider::Provider::ANTHROPIC)
       row.credentials_json = {"api_key" => JSON::Any.new("test-key")}

spec/llm_prompt_spec.cr

@@ -30,6 +30,14 @@ module PlaceOS::Api
 
       request.system.should contain "You edit SVG floor-plan maps"
       request.system.should contain "set_attr: target, name, value"
+      request.system.should contain "Preserve the apparent original font weight and style shown in the snapshot"
+      request.system.should contain "use bold only when the original label is bold"
+      request.system.should contain "set fill-opacity to 0.3 explicitly"
+      request.system.should contain "Preserve the furniture unless the instruction explicitly requests its removal"
+      request.system.should contain "expanding the canvas is part of that request"
+      request.system.should contain "Never crop the old viewBox"
+      request.system.should contain "A new room needs a readable name label"
+      request.system.should contain "make a usable doorway through that shared wall"
       request.schema.should_not be_nil
       request.model.should eq "gpt-5.4-mini"
       request.max_output_tokens.should eq MAP_AI_MAX_OUTPUT_TOKENS
@@ -64,7 +72,12 @@ module PlaceOS::Api
 
       kinds = request.parts.map { |part| part.is_a?(Llm::ImagePart) ? "image" : part.as(Llm::TextPart).text.lines.first }
       kinds.index!("image").should be > kinds.index!(&.starts_with?("SUBTREES"))
-      kinds.index!("image").should be < kinds.index!(&.starts_with?("ANNOTATIONS"))
+      annotations_position = request.parts.index! do |part|
+        part.is_a?(Llm::TextPart) && part.text.includes?("ANNOTATIONS drawn by the person")
+      end
+      kinds.index!("image").should be < annotations_position
+      request.text.should contain "SNAPSHOT bounds in user units:"
+      request.text.should contain "these may include annotations outside the map canvas"
       request.images.size.should eq 1
       request.text.should contain "the numbers match the badges on the snapshot"
     end
@@ -378,6 +391,36 @@ module PlaceOS::Api
       end
     end
 
+    it "only accepts finite canvas bounds on the indexed SVG root" do
+      canvas_context = JSON.parse(%({"index":[{"a":"e0","tag":"svg","kind":"root"},{"a":"e1","tag":"svg","kind":"art"},{"a":"e2","tag":"rect","kind":"art"}]}))
+      values = ["0 0 100 100", "-20 -30 120 130", "0,0,100,100", "0 0 -100 100", "0 0 100 0", "0 0 100", "0 rubbish 0 100 100", "0 0 NaN 100", "0 0 Infinity 100", "0 0 1e999 100"]
+      ops = values.map do |value|
+        JSON.parse({"op" => "set_attr", "target" => {"a" => "e0"}, "name" => "viewBox", "value" => value}.to_json)
+      end
+      ops << JSON.parse(%({"op":"set_attr","target":{"a":"e0"},"name":"viewBox","value":null}))
+      ops << JSON.parse(%({"op":"set_attr","target":{"a":"e1"},"name":"viewBox","value":"0 0 100 100"}))
+      ops << JSON.parse(%({"op":"set_attr","target":{"a":"e2"},"name":"viewBox","value":"0 0 100 100"}))
+      checked = Llm::Ops.check(ops, canvas_context)
+      checked.issues.keys.should eq (4..13).map { |n| "op-#{n}" }
+      checked.issues["op-12"].should eq "canvas bounds can only be changed on the SVG root"
+      checked.issues["op-13"].should eq "canvas bounds can only be changed on the SVG root"
+    end
+
+    it "only accepts canvas bounds that keep the old map and stay within four times its size" do
+      canvas_context = JSON.parse(%({"viewBox":[0,0,100,100],"index":[{"a":"e0","tag":"svg","kind":"root"}]}))
+      values = ["0 0 100 100", "-50 -50 200 200", "0 -100 100 200", "10 0 100 100", "0 0 90 100", "0 0 500 100", "-100 -100 300 300"]
+      ops = values.map do |value|
+        JSON.parse({"op" => "set_attr", "target" => {"a" => "e0"}, "name" => "viewBox", "value" => value}.to_json)
+      end
+      checked = Llm::Ops.check(ops, canvas_context)
+      checked.issues.should eq({
+        "op-4" => "the canvas may expand but cannot crop the existing map",
+        "op-5" => "the canvas may expand but cannot crop the existing map",
+        "op-6" => "the canvas expansion exceeds four times the existing size",
+        "op-7" => "the canvas expansion exceeds four times the existing size",
+      })
+    end
+
     it "resolves explicit ids created earlier in a proposal without accepting forward references" do
       created_context = JSON.parse(%({"index":[{"a":"e1","id":"chair","tag":"path"}]}))
       ops = [
@@ -411,6 +454,42 @@ module PlaceOS::Api
       checked.issues["op-1"].should eq "ai-chair-2 is already an id in the map"
     end
 
+    it "renumbers rooms sequentially and reuses the freed id for a new room" do
+      rename_context = JSON.parse(%({"index":[{"a":"e1","id":"room-04","tag":"rect"},{"a":"e2","id":"room-05","tag":"rect"}]}))
+      ops = [
+        %({"op":"set_id","target":{"a":"e2","id":"room-05"},"new_id":"room-06"}),
+        %({"op":"set_id","target":{"a":"e1","id":"room-04"},"new_id":"room-05"}),
+        %({"op":"insert_shape","kind":"rect","new_id":"room-04","attrs":[]}),
+        %({"op":"set_attr","target":{"id":"room-06"},"name":"fill","value":"red"}),
+      ].map { |op| JSON.parse(op) }
+      Llm::Ops.check(ops, rename_context).issues.should be_empty
+      rename_context["index"][0]["id"].as_s.should eq "room-04"
+      rename_context["index"][1]["id"].as_s.should eq "room-05"
+    end
+
+    it "swaps ids through a temporary id and releases earlier claimed ids" do
+      rename_context = JSON.parse(%({"index":[{"a":"e1","id":"zone-a","tag":"g"},{"a":"e2","id":"zone-b","tag":"g"}]}))
+      ops = [
+        %({"op":"set_id","target":{"a":"e1"},"new_id":"temp-zone"}),
+        %({"op":"set_id","target":{"a":"e2"},"new_id":"zone-a"}),
+        %({"op":"set_id","target":{"id":"temp-zone"},"new_id":"zone-b"}),
+        %({"op":"insert_shape","kind":"rect","new_id":"temp-zone","attrs":[]}),
+        %({"op":"insert_shape","kind":"rect","new_id":"zone-a","attrs":[]}),
+      ].map { |op| JSON.parse(op) }
+      Llm::Ops.check(ops, rename_context).issues.keys.should eq ["op-5"]
+    end
+
+    it "does not release an id after a rejected rename" do
+      rename_context = JSON.parse(%({"index":[{"a":"e1","id":"room-a","tag":"rect"},{"a":"e2","id":"room-b","tag":"rect"}]}))
+      ops = [
+        %({"op":"set_id","target":{"a":"e1"},"new_id":"room-b"}),
+        %({"op":"insert_shape","kind":"rect","new_id":"room-a","attrs":[]}),
+        %({"op":"set_id","target":{"a":"e1"},"new_id":null}),
+        %({"op":"insert_shape","kind":"rect","new_id":"room-a","attrs":[]}),
+      ].map { |op| JSON.parse(op) }
+      Llm::Ops.check(ops, rename_context).issues.keys.should eq ["op-1", "op-2"]
+    end
+
     it "flags everything past the op limit" do
       ops = Array.new(MAP_AI_MAX_OPS + 2) { JSON.parse(%({"op":"no_change","reason":"r","why":"w"})) }
       checked = Llm::Ops.check(ops, context)

src/placeos-rest-api/controllers/map_ai.cr

@@ -336,21 +336,12 @@ module PlaceOS::Api
       parsed = ::PlaceOS::Model::SignageAIJob::Kind.parse?(kind)
       raise Error::ModelValidation.new([Error::Field.new(:kind, "must be one of generate, edit, map_edit")]) if parsed.nil?
 
-      query = ::PlaceOS::Model::SignageAIJob
-        .where(authority_id: authority.id.as(String))
-        .where("kind = ?", ::PlaceOS::Model::SignageAIJob.kind_label(parsed))
-      if mine
-        query = query.where(user_id: current_user.id.as(String))
-      else
-        check_support
-      end
+      query = recent_ai_jobs([parsed], mine)
       if (zone = zone_id.presence)
         query = query.where("request->>'zone_id' = ?", zone)
       end
 
       query
-        .where("created_at > ?", 7.days.ago)
-        .order(created_at: :desc)
         .limit(limit)
         .to_a
         .map { |job| JobResponse.new(job) }
@@ -359,17 +350,7 @@ module PlaceOS::Api
     # ask a running job to stop. A call already with the vendor runs 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.save
-        ::PlaceOS::Model::SignageAIJob.bump_version(job.id.as(UUID))
-        job = ::PlaceOS::Model::SignageAIJob.find!(job.id.as(UUID))
-      end
-
-      JobResponse.new(job)
+      JobResponse.new(request_job_cancel(current_job))
     end
 
     # record what the person did with the answer: which ops were applied or
@@ -460,14 +441,7 @@ module PlaceOS::Api
 
     private def remaining_quota(row : ::PlaceOS::Model::SignageAIProvider?)
       user_limit, domain_limit = quotas_for(row)
-      kind = ::PlaceOS::Model::SignageAIJob::Kind::MapEdit
-      used_today = ::PlaceOS::Model::SignageAIJob.sum_candidates(current_user.id.as(String), 1.day.ago, kind)
-      used_month = ::PlaceOS::Model::SignageAIJob.sum_candidates_for_authority(authority.id.as(String), 30.days.ago, kind)
-
-      {
-        user_remaining_today:   Math.max(0, user_limit - used_today),
-        domain_remaining_month: Math.max(0, domain_limit - used_month),
-      }
+      remaining_ai_quota(::PlaceOS::Model::SignageAIJob::Kind::MapEdit, user_limit, domain_limit)
     end
 
     # The marked up snapshot: the caller's own image, under the size cap, tagged

src/placeos-rest-api/controllers/signage/ai.cr

@@ -274,20 +274,7 @@ module PlaceOS::Api
               else
                 ::PlaceOS::Model::SignageAIJob::Kind.image
               end
-      labels = kinds.map { |entry| ::PlaceOS::Model::SignageAIJob.kind_label(entry) }
-
-      query = ::PlaceOS::Model::SignageAIJob
-        .where(authority_id: authority.id.as(String))
-        .where("kind IN (#{labels.map { "?" }.join(", ")})", args: labels.map(&.as(::PgORM::Value)))
-      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)
+      recent_ai_jobs(kinds, mine)
         .limit(limit)
         .to_a
         .map { |job| JobResponse.new(job) }
@@ -296,20 +283,7 @@ module PlaceOS::Api
     # 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.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)
+      JobResponse.new(request_job_cancel(current_job))
     end
 
     # record that a candidate became a media item, so the sweep leaves it alone
@@ -397,13 +371,7 @@ module PlaceOS::Api
 
     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, ::PlaceOS::Model::SignageAIJob::Kind.image)
-      used_month = ::PlaceOS::Model::SignageAIJob.sum_candidates_for_authority(authority.id.as(String), 30.days.ago, ::PlaceOS::Model::SignageAIJob::Kind.image)
-
-      {
-        user_remaining_today:   Math.max(0, user_limit - used_today),
-        domain_remaining_month: Math.max(0, domain_limit - used_month),
-      }
+      remaining_ai_quota(::PlaceOS::Model::SignageAIJob::Kind.image, user_limit, domain_limit)
     end
 
     # An upload the caller may use as a source or a reference: one they own, or

src/placeos-rest-api/utilities/ai_jobs.cr

@@ -29,6 +29,48 @@ module PlaceOS::Api
       job
     end
 
+    # What the caller and the domain have left of their allowance, counting the
+    # candidates of jobs of `kinds` over the last day and 30 days.
+    def remaining_ai_quota(kinds : ::PlaceOS::Model::SignageAIJob::Kind | Array(::PlaceOS::Model::SignageAIJob::Kind), user_limit : Int32, domain_limit : Int32)
+      used_today = ::PlaceOS::Model::SignageAIJob.sum_candidates(current_user.id.as(String), 1.day.ago, kinds)
+      used_month = ::PlaceOS::Model::SignageAIJob.sum_candidates_for_authority(authority.id.as(String), 30.days.ago, kinds)
+
+      {
+        user_remaining_today:   Math.max(0, user_limit - used_today),
+        domain_remaining_month: Math.max(0, domain_limit - used_month),
+      }
+    end
+
+    # Flags the job for its runner to stop, unless it already finished. Only its
+    # owner or support may cancel. Returns the job as stored afterwards.
+    def request_job_cancel(job : ::PlaceOS::Model::SignageAIJob) : ::PlaceOS::Model::SignageAIJob
+      raise Error::Forbidden.new unless job.user_id == current_user.id || user_support?
+      return job if job.final?
+
+      job.cancel_requested = true
+      job.save
+      # in SQL, not read-modify-write: a runner 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))
+      ::PlaceOS::Model::SignageAIJob.find!(job.id.as(UUID))
+    end
+
+    # The domain's jobs of `kinds` from the last 7 days, newest first; only the
+    # caller's own unless `mine` is false, which needs support.
+    def recent_ai_jobs(kinds : Array(::PlaceOS::Model::SignageAIJob::Kind), mine : Bool)
+      labels = kinds.map { |entry| ::PlaceOS::Model::SignageAIJob.kind_label(entry) }
+      query = ::PlaceOS::Model::SignageAIJob
+        .where(authority_id: authority.id.as(String))
+        .where("kind IN (#{labels.map { "?" }.join(", ")})", args: labels.map(&.as(::PgORM::Value)))
+      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)
+    end
+
     # A job of this domain, 404 otherwise so ids from other domains read as unknown.
     def authority_job(id : UUID, authority_id : String) : ::PlaceOS::Model::SignageAIJob
       job = ::PlaceOS::Model::SignageAIJob.find!(id)

src/placeos-rest-api/utilities/image_gen/adapters/openai_images.cr

@@ -1,6 +1,8 @@
 require "http/formdata"
 require "json"
 
+require "../../openai_vendor"
+
 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.
@@ -9,8 +11,9 @@ module PlaceOS::Api::ImageGen::Adapters
   # 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
+    include Utils::OpenAIVendor
+
     DEFAULT_MODEL   = "gpt-image-2.5-sunburst"
-    DEFAULT_BASE    = "https://api.openai.com/v1"
     DEFAULT_VERSION = "2026-04-01-preview"
 
     # Sunburst is tuned for edit precision, Flare for speed; both cost the same.
@@ -122,20 +125,6 @@ module PlaceOS::Api::ImageGen::Adapters
       end
     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")
@@ -146,52 +135,16 @@ module PlaceOS::Api::ImageGen::Adapters
       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)
+        client.post(uri.request_target, headers: openai_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)