From 422e1fbcf2a33620b736a4b1d6b2f958c53ab389 Mon Sep 17 00:00:00 2001 From: Cameron Reeves Date: Thu, 15 Oct 2026 23:07:51 +1100 Subject: [PATCH 1/6] fix(map-ai): sequential renames, canvas expansion and SD replay prompt rules These changes were made on 14 Oct and ran in the rest-api sidecar for the local and mqt.placeos.run demos, but were never committed: - set_id renames update the index in proposal order, so later ops can target the new id and the old id becomes free - set_attr viewBox is allowed on the SVG root only, with four finite numbers - prompt rules for preserving label font weight, composited overlay contrast, referencing ids created earlier in the same proposal, translucent booking overlays, shared-wall doorways and bounded canvas expansion Co-authored-by: Codex Co-Authored-By: Claude Opus 5.5 --- spec/llm_prompt_spec.cr | 66 +++++++++++++++++++- src/placeos-rest-api/utilities/llm/ops.cr | 29 ++++++++- src/placeos-rest-api/utilities/llm/prompt.cr | 15 ++++- 3 files changed, 104 insertions(+), 6 deletions(-) diff --git a/spec/llm_prompt_spec.cr b/spec/llm_prompt_spec.cr index 4fe30a25..42181919 100644 --- a/spec/llm_prompt_spec.cr +++ b/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,21 @@ 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 "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 +439,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) diff --git a/src/placeos-rest-api/utilities/llm/ops.cr b/src/placeos-rest-api/utilities/llm/ops.cr index 9eb31efe..f4a1b339 100644 --- a/src/placeos-rest-api/utilities/llm/ops.cr +++ b/src/placeos-rest-api/utilities/llm/ops.cr @@ -75,6 +75,7 @@ module PlaceOS::Api::Llm if issue issues[id_for(position)] = issue elsif (hash = op.as_h?) + index.register_rename(hash, claimed) index.register_created(hash) end end @@ -92,7 +93,7 @@ module PlaceOS::Api::Llm @anchors = {} of String => Hash(String, JSON::Any) @ids = {} of String => Hash(String, JSON::Any) (context["index"]?.try(&.as_a?) || [] of JSON::Any).each do |entry| - hash = entry.as_h? + hash = entry.as_h?.try(&.dup) next unless hash if (anchor = hash["a"]?.try(&.as_s?)) @anchors[anchor] = hash @@ -128,6 +129,23 @@ module PlaceOS::Api::Llm {entry, nil} end + # Id ownership follows accepted renames in proposal order. + def register_rename(op : Hash(String, JSON::Any), claimed : Set(String)) : Nil + return unless op["op"]?.try(&.as_s?) == "set_id" + entry, _ = resolve(op["target"]?) + return unless entry + if (old_id = entry["id"]?.try(&.as_s?)) + @ids.delete(old_id) + claimed.delete(old_id) + end + if (new_id = op["new_id"]?.try(&.as_s?).presence) + entry["id"] = JSON::Any.new(new_id) + @ids[new_id] = entry + else + entry.delete("id") + end + end + # Later operations may reference explicit ids created earlier in this batch. def register_created(op : Hash(String, JSON::Any)) : Nil id = op["new_id"]?.try(&.as_s?).presence @@ -161,9 +179,16 @@ module PlaceOS::Api::Llm case kind when "set_attr" - with_target(op, index) do + with_target(op, index) do |entry| name = op["name"]?.try(&.as_s?).try(&.strip).presence return "set_attr needs a name" unless name + if name == "viewBox" + return "canvas bounds can only be changed on the SVG root" unless entry["kind"]?.try(&.as_s?) == "root" + tokens = op["value"]?.try(&.as_s?).try(&.strip.split(/[\s,]+/)) || [] of String + numbers = tokens.compact_map(&.to_f64?) + return "canvas bounds need four finite numbers with positive dimensions" unless tokens.size == 4 && numbers.size == 4 && numbers.all?(&.finite?) && numbers[2] > 0 && numbers[3] > 0 + return nil + end return "attribute #{name} may not be set" unless Schema.allowed_attribute?(name) if (value = op["value"]?.try(&.as_s?)) Values.attribute_value_issue(name, value) diff --git a/src/placeos-rest-api/utilities/llm/prompt.cr b/src/placeos-rest-api/utilities/llm/prompt.cr index f9c0f4f9..4bb94e67 100644 --- a/src/placeos-rest-api/utilities/llm/prompt.cr +++ b/src/placeos-rest-api/utilities/llm/prompt.cr @@ -26,7 +26,7 @@ module PlaceOS::Api::Llm Names and ids: rooms, desks, bays and lockers carry their number in their id (area-1.03-status is room 1.03, table-01.004 is desk 01.004, park-100 is bay 100). A name in the instruction such as "Boardroom 1.03" or "desk 01.004" identifies the element whose id carries that number; the words around the number are the label painted on the map, which the snapshot shows. - Labels: the LABELS section lists every label the application found, of two kinds. A text label is a real text element and its line shows its words: rename it with set_text on that element, keeping its lines (a newline character between lines), and never delete it to insert another. An outlined label is a word drawn as anonymous path elements, one per letter: its line shows the element it sits in (- when it is outside every room), its glyph anchors in reading order, its box and fill; the index marks those elements kind glyph, and the snapshot shows what the label says. To rename an outlined label: delete every glyph anchor of that label, then insert_shape a text element with the new words, after the label's last glyph (the application puts it where the glyphs were), text-anchor middle, x at the centre of the label's box, y at the baseline of the first line (the box's top plus the font size), font-size equal to the box height divided by the number of rows, font-family sans-serif, font-weight bold and the label's fill. Keep the label's rows: separate lines with a newline character in text and the application lays them out one under the other. + Labels: the LABELS section lists every label the application found, of two kinds. A text label is a real text element and its line shows its words: rename it with set_text on that element, keeping its lines (a newline character between lines), and never delete it to insert another. An outlined label is a word drawn as anonymous path elements, one per letter: its line shows the element it sits in (- when it is outside every room), its glyph anchors in reading order, its box and fill; the index marks those elements kind glyph, and the snapshot shows what the label says. To rename an outlined label: delete every glyph anchor of that label, then insert_shape a text element with the new words, after the label's last glyph (the application puts it where the glyphs were), text-anchor middle, x at the centre of the label's box, y at the baseline of the first line (the box's top plus the font size), font-size equal to the box height divided by the number of rows, font-family sans-serif and the label's fill. Preserve the apparent original font weight and style shown in the snapshot; use bold only when the original label is bold. Keep the label's rows: separate lines with a newline character in text and the application lays them out one under the other. Finding a label by its words: when the instruction quotes words rather than a number ("Kitchen", "Back Entry/Exit"), look for a text label with those words first; otherwise read the snapshot to see which outlined label shows them, matching each label's box to where the words are drawn. A label inside a room names that room; a label in a corridor or open area names the space around it. Act only on the label whose words match, never on another label nearby. @@ -38,19 +38,25 @@ module PlaceOS::Api::Llm - Refer to elements only by anchor or id exactly as they appear in the index, as a target object {"a": anchor, "id": id, "h": hash}. Never invent anchors or ids. Never estimate the coordinates of an existing element. - Prefer the smallest change that satisfies the instruction. Do not touch elements the instruction does not concern. - To move, resize or rotate an element use transform (user units); never rewrite its geometry. - - After changing a background, fill or overlay, check every label affected by that change against the final visible background. Keep labels unobscured and clearly readable, matching the high-contrast light-on-dark or dark-on-light treatment of neighbouring room labels. Recolour the affected text and tspans, or every glyph of an outlined label, with set_attr when needed; preserve the wording, position and geometry. This readability adjustment is part of the requested edit. Do not recolour a shared class or unrelated labels. Keep the label above the overlay in paint order. + - After changing a background, fill or overlay, check every label affected by that change against the final visible background. Keep labels unobscured and clearly readable, matching the high-contrast light-on-dark or dark-on-light treatment of neighbouring room labels. Recolour the affected text and tspans, or every glyph of an outlined label, with set_attr when needed; preserve the wording, position and geometry. This readability adjustment is part of the requested edit. Do not recolour a shared class or unrelated labels. Keep the label above the overlay in paint order. Evaluate the composited colour: a dark fill at 0.3 opacity over a light floor produces a light background, so use dark text (for example #24242b), not white. - To recolour one element use set_attr with name fill or stroke on that element. To restyle every element of a class use set_style_rule with a selector like .cls-12. Never touch the style rules unless asked. + - To reference an element created earlier in this same proposal, use its new_id as the target id with a and h null. New elements do not have index anchors yet; never use their id as an anchor. This also applies to parent and after references. - To add a copy of an existing element use duplicate with an offset. To draw a new shape use insert_shape with a parent anchor, the element kind and its attributes; derive new geometry from the annotation or from a named reference element. New ids must start with a letter, be unique, and follow the naming pattern of their neighbours; when nothing suggests a name, start the id with "ai-". - set_text only works on text or tspan elements; outlined labels are renamed as described above, never with set_text. - Chairs, plants and other furniture are anonymous shapes inside the room; find them by the inside column, their tag and fill, and their box. "The desk on the left in room 1.03" is the desk (table-... id) inside area-1.03-status with the smallest x. - A drawn object is usually several sibling elements stacked on one spot (a chair: its shadow, its seat and a stroke-only outline). Elements that share a piece value are the layers of one object: delete, move or copy every element of the piece together, with the same offset. A chair at a desk is one piece next to the desk's box; to add chairs, duplicate a whole piece per chair. - duplicate keeps the copy in the source element's parent group. For added furniture, choose a complete source piece in the destination room's furniture group; use transform on the copies to rotate them around the new piece centre if the requested orientation differs. Never place a copy in another room while leaving it in the source room's group. A shared furniture layer is suitable only when it already contains the destination furniture. Check the parent chain as well as the inside column. New elements can be targeted by their new_id in later operations. Keep all layers of each chair together and in their original paint order. - Desks come in runs. To add desks beside existing ones, duplicate the desk nearest the new position (its table-... element and the chair pieces beside it, all by one offset per new desk); the offset is the spacing between the two nearest desks of that run, so the copies continue the run on the side asked for, and new ids continue the neighbours' numbering (table-01.012 after table-01.011). Copy the desk next to where the new ones go, never a desk in another room, and keep the copies inside the room. - - A room is bookable when a rect or path with an area--status id lies over it, in the same parent as the other area-* elements. To make a room bookable, insert_shape such a rect into that parent covering the room (the extent of its walls, or of its label and furniture), with the class and attributes of a neighbouring area-* element and an id following their pattern. + - A room is bookable when a rect or path with an area--status id lies over it, in the same parent as the other area-* elements. To make a room bookable, insert_shape such a rect into that parent covering the room (the extent of its walls, or of its label and furniture), with an id following the neighbouring area-* pattern. New booking overlays must preserve visible desks, chairs and other furniture underneath: set fill-opacity to 0.3 explicitly, even when copying a neighbouring room's class. Never copy a solid room fill without adding this transparency. Preserve the furniture unless the instruction explicitly requests its removal. Check labels against the resulting translucent background and use readable contrasting text. Keep existing room overlays unchanged unless asked. - Ids marked protected are bound to bookings; delete, rename or hide them only when the instruction asks for exactly that. - Act on the best-supported reading and say in why what you took the words to mean. Return a single ask op only when no element carries the number the instruction names, when several do and nothing else separates them, or when a needed position cannot be derived from any element. When an instruction has several parts, do every part you can and name the rest in summary. If nothing needs to change, return no_change. - Every op carries a one-sentence why that names the element's label. summary is one sentence for the person stating only what the ops do, and names any part of the instruction that was not done. questions is usually empty. + - For a shared-wall doorway, use the wall markup supplied in the shortlist to cut the wall geometry itself, retaining all other segments and paint attributes. Do not cover a wall with a floor-coloured rectangle. + Coordinates: user units of the viewBox; x grows to the right, y grows downward. + - When the requested new space or its annotation lies outside the viewBox, expanding the canvas is part of that request: emit set_attr on the indexed SVG root with name viewBox and four numbers as its value before adding the room. Take the union of the existing viewBox and all new geometry, including half the wall stroke and a small margin. Keep every existing coordinate unchanged. Never crop the old viewBox or expand either dimension or total area beyond four times its current size. Do not ask for confirmation to show space the person already asked to add. + - A new room needs a readable name label. If no name was given, use the next room number following neighbouring labels (for example 1.05 after 1.04) and explain it in summary. Match neighbouring text styling, centred inside the new room, above its translucent booking overlay. + - When a new room adjoins an existing outside wall, make a usable doorway through that shared wall. Use the exact existing wall coordinates and a doorway width matching nearby openings. Preserve every other wall segment. A polyline can be replaced with an equivalent path containing separate subpaths on either side of the doorway; retain its original attributes. Keep the room floor connected to that opening and avoid drawing another wall across the entrance. If the annotation ends within one wall thickness of the existing wall, join the room to that wall. Fields each operation carries, besides op and why: TEXT @@ -231,6 +237,9 @@ module PlaceOS::Api::Llm String.build do |text| if markup + if with_image && (image_bounds = markup["viewBox"]?.try(&.as_a?)) + text << "SNAPSHOT bounds in user units: " << image_bounds.map { |value| number(value) }.join(' ') << "; these may include annotations outside the map canvas.\n" + end annotations = markup["annotations"]?.try(&.as_a?) || [] of JSON::Any unless annotations.empty? text << "ANNOTATIONS drawn by the person, in viewBox units" From 0e35de306d1cb936f8f65bbafae7ca25ecf1e4b2 Mon Sep 17 00:00:00 2001 From: Cameron Reeves Date: Thu, 15 Oct 2026 23:09:06 +1100 Subject: [PATCH 2/6] fix(map-ai): report a cut-off Anthropic proposal as length With the proposal returned as a tool input, a response that stopped at max_tokens either had no tool_use block, which raised InvalidResponse, or a truncated one. Both skipped the runner's retry with a larger output budget. The adapter now sets finish to length before reading the tool input. Co-Authored-By: Claude Opus 5.5 --- spec/llm_anthropic_spec.cr | 19 +++++++++++++++++++ .../llm/adapters/anthropic_messages.cr | 18 +++++++++++------- 2 files changed, 30 insertions(+), 7 deletions(-) diff --git a/spec/llm_anthropic_spec.cr b/spec/llm_anthropic_spec.cr index 7a8a4731..ca919c0d 100644 --- a/spec/llm_anthropic_spec.cr +++ b/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")} diff --git a/src/placeos-rest-api/utilities/llm/adapters/anthropic_messages.cr b/src/placeos-rest-api/utilities/llm/adapters/anthropic_messages.cr index 1fa3892e..7246a317 100644 --- a/src/placeos-rest-api/utilities/llm/adapters/anthropic_messages.cr +++ b/src/placeos-rest-api/utilities/llm/adapters/anthropic_messages.cr @@ -179,8 +179,18 @@ module PlaceOS::Api::Llm::Adapters ) end + finish = case stop + when "end_turn", "stop_sequence", "tool_use" then "stop" + when "max_tokens" then "length" + else "other" + end + + # A proposal cut off at max_tokens has no complete tool input; the runner + # retries "length" with a larger budget and never parses this text. blocks = payload["content"]?.try(&.as_a?) || [] of JSON::Any - text = if tool_name + text = if finish == "length" + "" + elsif tool_name calls = blocks.select { |block| block["type"]?.try(&.as_s?) == "tool_use" } call = calls.first? if calls.size != 1 || call.try(&.["name"]?).try(&.as_s?) != tool_name || !call.try(&.["input"]?).try(&.as_h?) @@ -193,12 +203,6 @@ module PlaceOS::Api::Llm::Adapters end.join end - finish = case stop - when "end_turn", "stop_sequence", "tool_use" then "stop" - when "max_tokens" then "length" - else "other" - end - usage = payload["usage"]? input = usage.try(&.["input_tokens"]?).try(&.as_i64?) output = usage.try(&.["output_tokens"]?).try(&.as_i64?) From ee22167f77bd317ddbb3aac5d6cdfbd657d02ae2 Mon Sep 17 00:00:00 2001 From: Cameron Reeves Date: Thu, 15 Oct 2026 23:09:06 +1100 Subject: [PATCH 3/6] fix(map-ai): check canvas expansion bounds on the server The server accepted any positive viewBox on the root, while the editor rejects one that crops the old map or grows it beyond four times. The op checker now applies the same rule against the viewBox the editor posts in the context, so the job's server issues agree with what the editor will allow. Co-Authored-By: Claude Opus 5.5 --- spec/llm_prompt_spec.cr | 15 +++++++++++++++ src/placeos-rest-api/utilities/llm/ops.cr | 21 ++++++++++++++++++++- 2 files changed, 35 insertions(+), 1 deletion(-) diff --git a/spec/llm_prompt_spec.cr b/spec/llm_prompt_spec.cr index 42181919..a3be4862 100644 --- a/spec/llm_prompt_spec.cr +++ b/spec/llm_prompt_spec.cr @@ -406,6 +406,21 @@ module PlaceOS::Api 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 = [ diff --git a/src/placeos-rest-api/utilities/llm/ops.cr b/src/placeos-rest-api/utilities/llm/ops.cr index f4a1b339..026d5195 100644 --- a/src/placeos-rest-api/utilities/llm/ops.cr +++ b/src/placeos-rest-api/utilities/llm/ops.cr @@ -88,6 +88,8 @@ module PlaceOS::Api::Llm getter anchors : Hash(String, Hash(String, JSON::Any)) getter ids : Hash(String, Hash(String, JSON::Any)) getter protected_ids : Set(String) + # The map's viewBox as x, y, width, height; nil when the context has none. + getter view_box : Array(Float64)? def initialize(context : JSON::Any) @anchors = {} of String => Hash(String, JSON::Any) @@ -103,6 +105,8 @@ module PlaceOS::Api::Llm end end @protected_ids = (context["protected_ids"]?.try(&.as_a?) || [] of JSON::Any).compact_map(&.as_s?).to_set + numbers = (context["viewBox"]?.try(&.as_a?) || [] of JSON::Any).compact_map { |value| value.as_f? || value.as_i64?.try(&.to_f) } + @view_box = numbers if numbers.size == 4 && numbers[2] > 0 && numbers[3] > 0 end # The entry a target names, or the reason it does not resolve. @@ -173,6 +177,21 @@ module PlaceOS::Api::Llm end end + # The editor's `canvasExpansionIssue`: the new bounds keep the whole old + # viewBox and grow neither dimension nor the area beyond four times. + def self.canvas_expansion_issue(before : Array(Float64)?, after : Array(Float64)) : String? + return nil unless before + bx, by, bw, bh = before + ax, ay, aw, ah = after + if ax > bx || ay > by || ax + aw < bx + bw || ay + ah < by + bh + return "the canvas may expand but cannot crop the existing map" + end + if aw > bw * 4 || ah > bh * 4 || aw * ah > bw * bh * 4 + return "the canvas expansion exceeds four times the existing size" + end + nil + end + private def self.issue_for(op : Hash(String, JSON::Any), index : Index, claimed : Set(String)) : String? kind = op["op"]?.try(&.as_s?) return "unknown operation #{kind.inspect}" unless kind && Schema::OPS.includes?(kind) @@ -187,7 +206,7 @@ module PlaceOS::Api::Llm tokens = op["value"]?.try(&.as_s?).try(&.strip.split(/[\s,]+/)) || [] of String numbers = tokens.compact_map(&.to_f64?) return "canvas bounds need four finite numbers with positive dimensions" unless tokens.size == 4 && numbers.size == 4 && numbers.all?(&.finite?) && numbers[2] > 0 && numbers[3] > 0 - return nil + return Ops.canvas_expansion_issue(index.view_box, numbers) end return "attribute #{name} may not be set" unless Schema.allowed_attribute?(name) if (value = op["value"]?.try(&.as_s?)) From 83c4edd177800265ce5f5131d0328ce893f1c717 Mon Sep 17 00:00:00 2001 From: Cameron Reeves Date: Thu, 15 Oct 2026 23:30:51 +1100 Subject: [PATCH 4/6] fix(map-ai): keep the snapshot bounds line under the annotations heading The bounds line was written ahead of the ANNOTATIONS heading, so the markup part no longer started with its heading and map_ai_spec could not find it. It explains annotations drawn off the map, so it now sits under that heading and is left out when there are no annotations. Co-Authored-By: Claude Opus 5.5 --- src/placeos-rest-api/utilities/llm/prompt.cr | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/placeos-rest-api/utilities/llm/prompt.cr b/src/placeos-rest-api/utilities/llm/prompt.cr index 4bb94e67..00f4fd2c 100644 --- a/src/placeos-rest-api/utilities/llm/prompt.cr +++ b/src/placeos-rest-api/utilities/llm/prompt.cr @@ -237,14 +237,14 @@ module PlaceOS::Api::Llm String.build do |text| if markup - if with_image && (image_bounds = markup["viewBox"]?.try(&.as_a?)) - text << "SNAPSHOT bounds in user units: " << image_bounds.map { |value| number(value) }.join(' ') << "; these may include annotations outside the map canvas.\n" - end annotations = markup["annotations"]?.try(&.as_a?) || [] of JSON::Any unless annotations.empty? text << "ANNOTATIONS drawn by the person, in viewBox units" text << "; the numbers match the badges on the snapshot" if with_image text << '\n' + if with_image && (image_bounds = markup["viewBox"]?.try(&.as_a?)) + text << "SNAPSHOT bounds in user units: " << image_bounds.map { |value| number(value) }.join(' ') << "; these may include annotations outside the map canvas.\n" + end used = 0 annotations.each do |mark| line = annotation_line(mark) From 26cae421d3bcf30e25270142e8ac73d766a01b2f Mon Sep 17 00:00:00 2001 From: Cameron Reeves Date: Thu, 15 Oct 2026 23:44:04 +1100 Subject: [PATCH 5/6] refactor(ai): share OpenAI and Azure plumbing between the image and map adapters OpenAIImages and the map edit OpenAIChat adapter each carried the same base URL, Azure check, authentication headers and error mapping. They now include Utils::OpenAIVendor. The image adapter also treats Azure's content_filter error code as a moderation refusal, as the chat adapter already did. Co-Authored-By: Claude Opus 5.5 --- .../image_gen/adapters/openai_images.cr | 57 ++-------------- .../utilities/llm/adapters/openai_chat.cr | 56 ++-------------- .../utilities/openai_vendor.cr | 67 +++++++++++++++++++ 3 files changed, 76 insertions(+), 104 deletions(-) create mode 100644 src/placeos-rest-api/utilities/openai_vendor.cr 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 3f8ce57b..9db7abcb 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 @@ -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) diff --git a/src/placeos-rest-api/utilities/llm/adapters/openai_chat.cr b/src/placeos-rest-api/utilities/llm/adapters/openai_chat.cr index 06806fd1..94865c22 100644 --- a/src/placeos-rest-api/utilities/llm/adapters/openai_chat.cr +++ b/src/placeos-rest-api/utilities/llm/adapters/openai_chat.cr @@ -1,6 +1,8 @@ require "base64" require "json" +require "../../openai_vendor" + module PlaceOS::Api::Llm::Adapters # OpenAI Chat Completions, and Azure OpenAI, which speaks the same request and # response shape at a different base URL with a different auth header. @@ -9,7 +11,7 @@ module PlaceOS::Api::Llm::Adapters # `/openai/v1` is the v1 API (no api-version); any other endpoint is the # legacy per-deployment URL with `credentials.api_version`. class OpenAIChat < Adapter - DEFAULT_BASE = "https://api.openai.com/v1" + include Utils::OpenAIVendor # the legacy Azure chat completions API version used when the row's # credentials name none @@ -93,20 +95,6 @@ module PlaceOS::Api::Llm::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(model : String) : URI if azure? && !base.ends_with?("/openai/v1") version = credentials["api_version"]?.try(&.as_s?).presence || DEFAULT_VERSION @@ -116,47 +104,11 @@ module PlaceOS::Api::Llm::Adapters end end - private def headers : HTTP::Headers - headers = HTTP::Headers{"Content-Type" => "application/json"} - 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(model : String, body : String) : HTTP::Client::Response uri = url_for(model) ImageGen::Http.client(uri, MAP_AI_READ_TIMEOUT) do |client| - client.post(uri.request_target, headers: headers, body: body) - end - 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 + client.post(uri.request_target, headers: openai_headers("application/json"), body: body) end - - if code == "content_filter" || 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) : Result diff --git a/src/placeos-rest-api/utilities/openai_vendor.cr b/src/placeos-rest-api/utilities/openai_vendor.cr new file mode 100644 index 00000000..682464ac --- /dev/null +++ b/src/placeos-rest-api/utilities/openai_vendor.cr @@ -0,0 +1,67 @@ +require "http/client" +require "json" + +module PlaceOS::Api + # Endpoint, authentication and error mapping shared by the OpenAI and Azure + # OpenAI adapters, for images and for map edits. + # + # The including class exposes the provider row as `row` and includes + # `Utils::VendorCredentials`. + module Utils::OpenAIVendor + OPENAI_BASE = "https://api.openai.com/v1" + + # error codes OpenAI and Azure use when a safety system blocked the request + MODERATION_CODES = {"content_filter", "moderation_blocked", "content_policy_violation"} + + protected def azure? : Bool + row.provider.azure_openai? + end + + # the row's endpoint without a trailing slash; Azure rows must set one + protected 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 + OPENAI_BASE + end + end + + protected def openai_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 + + protected 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_CODES.includes?(code) + 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 + end +end From 3450a358e9ee42872bc31dedc59b2dabd0165164 Mon Sep 17 00:00:00 2001 From: Cameron Reeves Date: Thu, 15 Oct 2026 23:44:32 +1100 Subject: [PATCH 6/6] refactor(ai): share quota, cancel and job listing between the AI controllers The map AI controller copied the signage AI controller's quota count, cancel action and recent-jobs query, differing only in the job kinds. They now live in Utils::AIJobs next to the shared long poll: remaining_ai_quota, request_job_cancel and recent_ai_jobs. Co-Authored-By: Claude Opus 5.5 --- src/placeos-rest-api/controllers/map_ai.cr | 32 ++------------ .../controllers/signage/ai.cr | 38 ++--------------- src/placeos-rest-api/utilities/ai_jobs.cr | 42 +++++++++++++++++++ 3 files changed, 48 insertions(+), 64 deletions(-) diff --git a/src/placeos-rest-api/controllers/map_ai.cr b/src/placeos-rest-api/controllers/map_ai.cr index 25bb87a6..6c87a4df 100644 --- a/src/placeos-rest-api/controllers/map_ai.cr +++ b/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 diff --git a/src/placeos-rest-api/controllers/signage/ai.cr b/src/placeos-rest-api/controllers/signage/ai.cr index 3e686597..3e88c941 100644 --- a/src/placeos-rest-api/controllers/signage/ai.cr +++ b/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 diff --git a/src/placeos-rest-api/utilities/ai_jobs.cr b/src/placeos-rest-api/utilities/ai_jobs.cr index f47be6c0..d7d4456e 100644 --- a/src/placeos-rest-api/utilities/ai_jobs.cr +++ b/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)