From 824545786e02409245e70e9ecd67b28a02226ec7 Mon Sep 17 00:00:00 2001 From: Cameron Reeves Date: Wed, 21 Oct 2026 12:22:00 +1100 Subject: [PATCH 1/3] fix(map-ai): accept dominant-baseline on inserted text (PPT-2832) GPT-6.1 sol adds dominant-baseline="middle" to every label it inserts. The server rejected the attribute, so each such insert failed even though its position was right. Testing on placeos-dev on 21 Oct lost 4 of 4 labels on both runs. The value is limited to the SVG 2 keywords plus text-before-edge and text-after-edge. ops-schema.json stays identical to the editor's copy in user-interfaces. Co-Authored-By: Claude Opus 5.5 --- spec/llm_prompt_spec.cr | 2 ++ src/placeos-rest-api/utilities/llm/ops-schema.json | 2 +- src/placeos-rest-api/utilities/llm/schema.cr | 3 ++- src/placeos-rest-api/utilities/llm/values.cr | 6 ++++++ 4 files changed, 11 insertions(+), 2 deletions(-) diff --git a/spec/llm_prompt_spec.cr b/spec/llm_prompt_spec.cr index 97199ef4..c84e05e8 100644 --- a/spec/llm_prompt_spec.cr +++ b/spec/llm_prompt_spec.cr @@ -230,6 +230,8 @@ module PlaceOS::Api Llm::Values.attribute_value_issue("font-weight", "700").should be_nil Llm::Values.attribute_value_issue("font-weight", "heavy").should eq %("heavy" is not a font weight) Llm::Values.attribute_value_issue("text-anchor", "middle").should be_nil + Llm::Values.attribute_value_issue("dominant-baseline", "middle").should be_nil + Llm::Values.attribute_value_issue("dominant-baseline", "centre").should eq %("centre" is not a dominant baseline) Llm::Values.attribute_value_issue("data-name", "Boardroom").should be_nil Llm::Values.attribute_value_issue("data-name", "x").should eq "data-name may not contain markup" end diff --git a/src/placeos-rest-api/utilities/llm/ops-schema.json b/src/placeos-rest-api/utilities/llm/ops-schema.json index 791efac7..bdddc75a 100644 --- a/src/placeos-rest-api/utilities/llm/ops-schema.json +++ b/src/placeos-rest-api/utilities/llm/ops-schema.json @@ -33,7 +33,7 @@ }, "name": { "type": "string", - "description": "fill, stroke, stroke-width, stroke-dasharray, fill-opacity, stroke-opacity, opacity, class, visibility, rx, ry, font-size, font-family, font-weight, text-anchor or data-*" + "description": "fill, stroke, stroke-width, stroke-dasharray, fill-opacity, stroke-opacity, opacity, class, visibility, rx, ry, font-size, font-family, font-weight, text-anchor, dominant-baseline or data-*" }, "value": { "type": ["string", "null"], diff --git a/src/placeos-rest-api/utilities/llm/schema.cr b/src/placeos-rest-api/utilities/llm/schema.cr index d3960826..6bd54b66 100644 --- a/src/placeos-rest-api/utilities/llm/schema.cr +++ b/src/placeos-rest-api/utilities/llm/schema.cr @@ -27,7 +27,8 @@ module PlaceOS::Api::Llm # attributes set_attr may touch, plus data-*; style rules take the same list ATTRIBUTES = %w(fill stroke stroke-width stroke-dasharray fill-opacity stroke-opacity - opacity class visibility rx ry font-size font-family font-weight text-anchor) + opacity class visibility rx ry font-size font-family font-weight text-anchor + dominant-baseline) DATA_ATTRIBUTE = /\Adata-[a-z][a-z0-9_-]*\z/i diff --git a/src/placeos-rest-api/utilities/llm/values.cr b/src/placeos-rest-api/utilities/llm/values.cr index aa376238..43d04df1 100644 --- a/src/placeos-rest-api/utilities/llm/values.cr +++ b/src/placeos-rest-api/utilities/llm/values.cr @@ -39,6 +39,10 @@ module PlaceOS::Api::Llm FONT_WEIGHT = /\A(?:normal|bold|bolder|lighter|[1-9]00)\z/ UNSAFE_TEXT = /url\(|expression\(|behavior:|javascript:|<|>|[\r\n]/i + # `dominant-baseline` keywords from SVG 2, plus the two SVG 1.1 edge keywords browsers still honour + DOMINANT_BASELINES = %w(auto alphabetic central hanging ideographic mathematical middle + text-after-edge text-before-edge text-bottom text-top) + # `sanitise.ts` UNSAFE_CSS_VALUE plus a backslash: browsers decode CSS # escapes (`u\72l(`), so escaped text cannot be matched literally UNSAFE_CSS = /expression\s*\(|behavior\s*:|javascript\s*:|url\s*\(\s*(?!['"]?#)|\\/i @@ -115,6 +119,8 @@ module PlaceOS::Api::Llm FONT_WEIGHT.match(value.strip) ? nil : "#{value.inspect} is not a font weight" when "text-anchor" %w(start middle end).includes?(value.strip) ? nil : "#{value.inspect} is not a text anchor" + when "dominant-baseline" + DOMINANT_BASELINES.includes?(value.strip) ? nil : "#{value.inspect} is not a dominant baseline" else if name.starts_with?("data-") && value =~ /[<>]/ "#{name} may not contain markup" From 529787974ff786c5035f1ce13a5ac0706df39d83 Mon Sep 17 00:00:00 2001 From: Cameron Reeves Date: Wed, 21 Oct 2026 12:35:51 +1100 Subject: [PATCH 2/3] feat(map-ai): ask the model once more about the ops the server rejects (PPT-2832) When the server rejects some of an answer's ops, the runner now makes one more call inside the same job. It sends the same request plus the answer's ops and the reason for each rejection, and asks for the whole proposal corrected. The second answer replaces the first only if it is complete, parses, keeps at least as many accepted ops, and has fewer rejected. If that call fails, the first answer stands. The job's `repair` field records how many ops were rejected, how many remain and whether the answer was replaced. MAP_AI_REPAIR=false turns it off. The server now also checks the parent/after rule the editor applies: `after` must be a direct child of `parent` (the root when none is named), and a parent must be the root or a . On placeos-dev, gpt-5.4-mini broke this with every label it inserted, and only the browser caught it, so no repair round could have fixed it. The check steps aside where the index cannot settle it: elements made earlier in the batch, an `after` deleted earlier in it, or a tree reshaped by reorder, group, ungroup or replace_element. Co-Authored-By: Claude Opus 5.5 --- OPENAPI_DOC.yml | 4 + spec/controllers/map_ai_spec.cr | 82 ++++++++++++++++++ spec/llm_prompt_spec.cr | 91 ++++++++++++++++++++ spec/spec_helpers/http_mocks.cr | 16 ++++ src/constants.cr | 3 + src/placeos-rest-api/controllers/map_ai.cr | 5 ++ src/placeos-rest-api/utilities/llm/ops.cr | 51 ++++++++++- src/placeos-rest-api/utilities/llm/prompt.cr | 19 ++++ src/placeos-rest-api/utilities/llm/runner.cr | 54 +++++++++++- 9 files changed, 320 insertions(+), 5 deletions(-) diff --git a/OPENAPI_DOC.yml b/OPENAPI_DOC.yml index 0bdef13f..be46dc1b 100644 --- a/OPENAPI_DOC.yml +++ b/OPENAPI_DOC.yml @@ -47306,6 +47306,10 @@ components: anyOf: - type: object - type: "null" + repair: + anyOf: + - type: object + - type: "null" resolution: anyOf: - type: object diff --git a/spec/controllers/map_ai_spec.cr b/spec/controllers/map_ai_spec.cr index e0ab0270..63ce901f 100644 --- a/spec/controllers/map_ai_spec.cr +++ b/spec/controllers/map_ai_spec.cr @@ -372,6 +372,88 @@ module PlaceOS::Api ops[0].as_h.has_key?("server_issue").should be_false final["server_issues"].as_h.keys.should eq ["op-1"] final["server_issues"]["op-1"].as_s.should eq "anchor e404 is not in the index" + # the repair round answered the same, so the first answer stands + final["repair"].should eq JSON.parse(%({"rejected":1,"remaining":1,"used":false})) + final["usage"]["attempts"].as_i.should eq 2 + end + + it "asks once more with the rejected ops and keeps the corrected answer" do + setup_signage_ai + _, headers = Spec::Authentication.authentication + level = map_ai_level + requests = [] of JSON::Any + + first = { + summary: "s", + ops: [ + {op: "set_attr", why: "w", target: {a: "e404", id: nil, h: nil}, name: "fill", value: "#fff"}, + {op: "set_attr", why: "w", target: {a: "e5", id: nil, h: nil}, name: "fill", value: "#fff"}, + ], + questions: [] of String, + }.to_json + second = { + summary: "s2", + ops: [ + {op: "set_attr", why: "w", target: {a: "e4", id: nil, h: nil}, name: "fill", value: "#fff"}, + {op: "set_attr", why: "w", target: {a: "e5", id: nil, h: nil}, name: "fill", value: "#fff"}, + ], + questions: [] of String, + }.to_json + HttpMocks.map_ai_vendor_sequence([first, second], requests) + + result = client.post(File.join(base, "edit"), headers: headers, body: {zone_id: level.id, instruction: "x", context: map_ai_context}.to_json) + final = await_signage_ai_job(base, JSON.parse(result.body)["id"].as_s, headers) + final["state"].as_s.should eq "done" + final["summary"].as_s.should eq "s2" + final["ops"].as_a.map(&.["target"]["a"].as_s).should eq ["e4", "e5"] + final["server_issues"].as_h.should be_empty + final["repair"].should eq JSON.parse(%({"rejected":1,"remaining":0,"used":true})) + final["usage"]["attempts"].as_i.should eq 2 + final["cost_units"].as_f.should eq 2560.0 + + requests.size.should eq 2 + asked_again = requests[1].to_json + asked_again.should contain "REJECTED" + asked_again.should contain "op-1: anchor e404 is not in the index" + end + + it "keeps the first answer when the repair would drop ops the server accepted" do + setup_signage_ai + _, headers = Spec::Authentication.authentication + level = map_ai_level + requests = [] of JSON::Any + + first = { + summary: "s", + ops: [ + {op: "set_attr", why: "w", target: {a: "e404", id: nil, h: nil}, name: "fill", value: "#fff"}, + {op: "set_attr", why: "w", target: {a: "e5", id: nil, h: nil}, name: "fill", value: "#fff"}, + ], + questions: [] of String, + }.to_json + HttpMocks.map_ai_vendor_sequence([first, {summary: "gave up", ops: [] of String, questions: [] of String}.to_json], requests) + + result = client.post(File.join(base, "edit"), headers: headers, body: {zone_id: level.id, instruction: "x", context: map_ai_context}.to_json) + final = await_signage_ai_job(base, JSON.parse(result.body)["id"].as_s, headers) + final["state"].as_s.should eq "done" + final["summary"].as_s.should eq "s" + final["ops"].as_a.size.should eq 2 + final["repair"].should eq JSON.parse(%({"rejected":1,"remaining":0,"used":false})) + requests.size.should eq 2 + end + + it "does not ask again when every op passes" do + setup_signage_ai + _, headers = Spec::Authentication.authentication + level = map_ai_level + requests = [] of JSON::Any + HttpMocks.map_ai_vendor(MAP_AI_ANSWER, requests: requests) + + result = client.post(File.join(base, "edit"), headers: headers, body: {zone_id: level.id, instruction: "x", context: map_ai_context}.to_json) + final = await_signage_ai_job(base, JSON.parse(result.body)["id"].as_s, headers) + final["state"].as_s.should eq "done" + final.as_h.has_key?("repair").should be_false + requests.size.should eq 1 end it "records a refusal as a moderation failure" do diff --git a/spec/llm_prompt_spec.cr b/spec/llm_prompt_spec.cr index c84e05e8..ba21786d 100644 --- a/spec/llm_prompt_spec.cr +++ b/spec/llm_prompt_spec.cr @@ -110,6 +110,36 @@ module PlaceOS::Api text.should_not contain "recolours Meeting 1.01" end + it "asks again with the rejected operations and why, after the instruction" do + request = Llm::Prompt.build(Llm::Prompt::Options.new( + context: context, + instruction: "Label Meeting 1.01.", + model: "gpt-6.1-sol", + )) + envelope = Llm::Ops::Envelope.new( + summary: "Labelled Meeting 1.01.", + ops: [ + JSON.parse(%({"op":"set_attr","target":{"a":"e2","id":null,"h":null},"name":"fill","value":"#90ee90","why":"w"})), + JSON.parse(%({"op":"insert_shape","parent":null,"after":{"a":"e2","id":null,"h":null},"kind":"text","attrs":[],"new_id":"ai-label","text":"1.01","why":"w"})), + ], + questions: [] of String, + ) + + repair = Llm::Prompt.repair(request, envelope, {"op-2" => "after must be a child of the parent"}) + + repair.parts.size.should eq request.parts.size + 1 + repair.parts[0...request.parts.size].should eq request.parts + repair.system.should eq request.system + repair.max_output_tokens.should eq request.max_output_tokens + text = repair.text + text.index!("REPAIR\n").should be > text.index!("INSTRUCTION\n") + text.should contain "PREVIOUS ANSWER summary: Labelled Meeting 1.01." + text.should contain %("op_id":"op-1","op":"set_attr") + text.should contain %("op_id":"op-2","op":"insert_shape") + text.should_not contain %("parent":null) + text.should contain "REJECTED\nop-2: after must be a child of the parent\n" + end + it "caps the index, the subtrees and the history" do big = JSON.parse(context.to_json) index = big["index"].as_a @@ -339,6 +369,9 @@ module PlaceOS::Api { %({"op":"insert_shape","parent":{"a":"e1","id":null,"h":null},"after":null,"kind":"path","attrs":[{"name":"d","value":"M0 0 L10 10 Z"}],"new_id":null,"text":null,"why":"w"}), nil }, { %({"op":"insert_shape","parent":{"a":"e1","id":null,"h":null},"after":null,"kind":"polygon","attrs":[{"name":"points","value":"0,0 10,0 10"}],"new_id":null,"text":null,"why":"w"}), %("0,0 10,0 10" is not a points list) }, { %({"op":"insert_shape","parent":{"a":"e99","id":null,"h":null},"after":null,"kind":"g","attrs":[],"new_id":null,"text":null,"why":"w"}), "parent: anchor e99 is not in the index" }, + { %({"op":"insert_shape","parent":{"a":"e1","id":null,"h":null},"after":{"a":"e2","id":null,"h":null},"kind":"rect","attrs":[],"new_id":null,"text":null,"why":"w"}), nil }, + { %({"op":"insert_shape","parent":null,"after":{"a":"e2","id":null,"h":null},"kind":"rect","attrs":[],"new_id":null,"text":null,"why":"w"}), "after must be a child of the parent" }, + { %({"op":"insert_shape","parent":{"a":"e5","id":null,"h":null},"after":null,"kind":"rect","attrs":[],"new_id":null,"text":null,"why":"w"}), "parent must be the root or a , not " }, { %({"op":"set_style_rule","selector":".cls-12","declarations":[{"name":"fill","value":"#eee"},{"name":"stroke","value":"#333"}],"why":"w"}), nil }, { %({"op":"set_style_rule","selector":"*","declarations":[{"name":"fill","value":"#eee"}],"why":"w"}), %(the selector "*" is not allowed) }, { %({"op":"set_style_rule","selector":".a{} @import url(http://evil); .b","declarations":[{"name":"fill","value":"#eee"}],"why":"w"}), %(the selector ".a{} @import url(http://evil); .b" is not allowed) }, @@ -468,6 +501,64 @@ module PlaceOS::Api checked.issues["op-1"].should eq "ai-chair-2 is already an id in the map" end + it "places an insert after a child of its parent, the root when none is named" do + tree = JSON.parse(%({"index":[ + {"a":"e0","id":"Layer_1","tag":"svg","p":null}, + {"a":"e1","id":"room_meeting","tag":"g","p":null}, + {"a":"e2","id":"room_border_8","tag":"g","p":"e1"}, + {"a":"e3","id":"space_room_7","tag":"rect","p":"e2"}, + {"a":"e4","id":"table","tag":"rect","p":"e1"} + ]})) + insert = ->(parent : String?, after : String?) do + JSON.parse({ + "op" => "insert_shape", "kind" => "text", "text" => "Sydney Room 1", "attrs" => [] of String, "new_id" => nil, + "parent" => parent.try { |anchor| {"a" => anchor} }, "after" => after.try { |anchor| {"a" => anchor} }, + }.to_json) + end + ops = [ + insert.call("e2", "e3"), # after a child of the parent + insert.call("e1", "e3"), # after a grandchild: the gpt-5.4-mini mistake + insert.call(nil, "e1"), # after a child of the root, no parent named + insert.call("e0", "e1"), # the root named as the parent + insert.call(nil, "e3"), # not a child of the root + insert.call("e3", nil), # a rect is no parent + insert.call("e1", nil), + ] + checked = Llm::Ops.check(ops, tree) + checked.issues.should eq({ + "op-2" => "after must be a child of the parent", + "op-5" => "after must be a child of the parent", + "op-6" => "parent must be the root or a , not ", + }) + end + + it "leaves the after check to the editor once the batch deletes after or moves elements" do + tree = JSON.parse(%({"index":[ + {"a":"e1","id":"room","tag":"g","p":null}, + {"a":"e2","id":"border","tag":"g","p":"e1"}, + {"a":"e3","id":"space","tag":"rect","p":"e2"}, + {"a":"e4","id":"label","tag":"text","p":"e1"} + ]})) + deleted = [ + %({"op":"delete","target":{"a":"e3"}}), + %({"op":"insert_shape","kind":"text","text":"x","attrs":[],"parent":{"a":"e1"},"after":{"a":"e3"}}), + %({"op":"insert_shape","kind":"text","text":"x","attrs":[],"parent":{"a":"e2"},"after":{"a":"e4"}}), + ].map { |op| JSON.parse(op) } + Llm::Ops.check(deleted, tree).issues.keys.should eq ["op-3"] + + moved = [ + %({"op":"group","targets":[{"a":"e3"},{"a":"e4"}],"new_id":"ai-group"}), + %({"op":"insert_shape","kind":"text","text":"x","attrs":[],"parent":{"a":"e2"},"after":{"a":"e4"}}), + ].map { |op| JSON.parse(op) } + Llm::Ops.check(moved, tree).issues.should be_empty + + created = [ + %({"op":"insert_shape","kind":"g","attrs":[],"new_id":"ai-wrap","parent":{"a":"e1"}}), + %({"op":"insert_shape","kind":"text","text":"x","attrs":[],"parent":{"id":"ai-wrap"},"after":{"a":"e4"}}), + ].map { |op| JSON.parse(op) } + Llm::Ops.check(created, tree).issues.should be_empty + 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 = [ diff --git a/spec/spec_helpers/http_mocks.cr b/spec/spec_helpers/http_mocks.cr index 643846fa..0e3fc50f 100644 --- a/spec/spec_helpers/http_mocks.cr +++ b/spec/spec_helpers/http_mocks.cr @@ -106,6 +106,22 @@ module PlaceOS::Api::HttpMocks end end + # `contents` answered in turn, the last one again once they run out. + def self.map_ai_vendor_sequence(contents : Array(String), requests : Array(JSON::Any)) + WebMock + .stub(:post, CHAT_COMPLETIONS) + .to_return do |request| + requests << JSON.parse(WebMock.body(request).to_s) + content = contents[Math.min(requests.size, contents.size) - 1] + body = { + id: "chatcmpl-spec", + choices: [{message: {role: "assistant", content: content, refusal: nil}, finish_reason: "stop"}], + usage: {prompt_tokens: 1200, completion_tokens: 80, total_tokens: 1280}, + }.to_json + HTTP::Client::Response.new(200, body, HTTP::Headers{"Content-Type" => "application/json"}) + end + end + # The model declining to answer, which the adapter maps to `Moderated`. def self.map_ai_vendor_refused body = { diff --git a/src/constants.cr b/src/constants.cr index 14e23524..0f5b2d67 100644 --- a/src/constants.cr +++ b/src/constants.cr @@ -78,6 +78,9 @@ module PlaceOS::Api # the second attempt's output budget after an answer cut off at MAP_AI_MAX_OUTPUT_TOKENS MAP_AI_RETRY_OUTPUT_TOKENS = (ENV["MAP_AI_RETRY_OUTPUT_TOKENS"]? || "24000").to_i + # one more vendor call, inside the same job, when the server rejects some of an answer's ops + MAP_AI_REPAIR = ENV["MAP_AI_REPAIR"]?.try(&.downcase) != "false" + # default quotas in calls, overridden per provider row by map_user_per_day # and map_domain_per_month MAP_AI_USER_PER_DAY = (ENV["MAP_AI_USER_PER_DAY"]? || "40").to_i diff --git a/src/placeos-rest-api/controllers/map_ai.cr b/src/placeos-rest-api/controllers/map_ai.cr index 6c87a4df..a19c1e72 100644 --- a/src/placeos-rest-api/controllers/map_ai.cr +++ b/src/placeos-rest-api/controllers/map_ai.cr @@ -115,6 +115,10 @@ module PlaceOS::Api getter server_issues : Hash(String, String) getter questions : Array(JSON::Any) getter usage : JSON::Any? + # when the server asked again about rejected ops: `rejected` before, + # `remaining` in the second answer (null when it was unusable), `used` + # when that answer replaced the first + getter repair : JSON::Any? getter resolution : JSON::Any? getter error_kind : String? getter error_message : String? @@ -145,6 +149,7 @@ module PlaceOS::Api end @questions = result["questions"]?.try(&.as_a?) || [] of JSON::Any @usage = result["usage"]? + @repair = result["repair"]? @resolution = result["resolution"]? @error_kind = job.error_kind @error_message = job.error_message diff --git a/src/placeos-rest-api/utilities/llm/ops.cr b/src/placeos-rest-api/utilities/llm/ops.cr index 753acb34..e4629054 100644 --- a/src/placeos-rest-api/utilities/llm/ops.cr +++ b/src/placeos-rest-api/utilities/llm/ops.cr @@ -105,6 +105,7 @@ module PlaceOS::Api::Llm elsif (hash = op.as_h?) index.register_rename(hash, claimed) index.register_created(hash) + index.register_structure(hash) end end @@ -118,6 +119,10 @@ module PlaceOS::Api::Llm 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)? + # anchors an earlier op in the batch deletes; an insert placed after one goes where it was + getter deleted = Set(String).new + # set once an earlier op in the batch can move elements to another parent + getter? restructured = false def initialize(context : JSON::Any) @anchors = {} of String => Hash(String, JSON::Any) @@ -203,6 +208,41 @@ module PlaceOS::Api::Llm def known_id?(id : String) : Bool @ids.has_key?(id) end + + # Records what an accepted op does to the tree that later insert_shape + # checks rely on. + def register_structure(op : Hash(String, JSON::Any)) : Nil + case op["op"]?.try(&.as_s?) + when "delete" + targets = op["targets"]?.try(&.as_a?) || [] of JSON::Any + ([op["target"]?] + targets).each do |target| + entry, _ = resolve(target) + if (anchor = entry.try(&.["a"]?).try(&.as_s?)) + @deleted << anchor + end + end + when "reorder", "group", "ungroup", "replace_element" + @restructured = true + end + end + + # The editor's rule for insert_shape: `after` is a direct child of + # `parent`, the root when none is named. Nil when it holds or cannot be + # judged from the index: `after` or `parent` made in this batch, `after` + # deleted earlier in it, or the tree restructured by an earlier op. + def sibling_issue(parent : Hash(String, JSON::Any)?, after : Hash(String, JSON::Any)) : String? + return nil if restructured? + return nil unless after.has_key?("p") + anchor = after["a"]?.try(&.as_s?) + return nil if anchor && @deleted.includes?(anchor) + + expected = nil + if parent && parent["tag"]?.try(&.as_s?) != "svg" + expected = parent["a"]?.try(&.as_s?) + return nil unless expected + end + after["p"]?.try(&.as_s?) == expected ? nil : "after must be a child of the parent" + end end # The editor's `canvasExpansionIssue`: the new bounds keep the whole old @@ -290,13 +330,20 @@ module PlaceOS::Api::Llm shape = op["kind"]?.try(&.as_s?) return "insert_shape needs a kind" unless shape return "#{shape} is not a shape that can be inserted" unless Schema::SHAPES.includes?(shape) + parent = nil if present?(op["parent"]?) - _, issue = index.resolve(op["parent"]) + parent, issue = index.resolve(op["parent"]) return "parent: #{issue}" if issue end + if (tag = parent.try(&.["tag"]?).try(&.as_s?)) && !{"g", "svg"}.includes?(tag) + return "parent must be the root or a , not <#{tag}>" + end if present?(op["after"]?) - _, issue = index.resolve(op["after"]) + after, issue = index.resolve(op["after"]) return "after: #{issue}" if issue + if after && (sibling = index.sibling_issue(parent, after)) + return sibling + end end geometry = Schema::SHAPE_GEOMETRY_ATTRIBUTES[shape] (op["attrs"]?.try(&.as_a?) || [] of JSON::Any).each do |attr| diff --git a/src/placeos-rest-api/utilities/llm/prompt.cr b/src/placeos-rest-api/utilities/llm/prompt.cr index 00f4fd2c..351f56cd 100644 --- a/src/placeos-rest-api/utilities/llm/prompt.cr +++ b/src/placeos-rest-api/utilities/llm/prompt.cr @@ -285,6 +285,25 @@ module PlaceOS::Api::Llm parts.join(' ') end + # `request` again with its answer's rejected operations and the reasons, + # asking for the whole proposal corrected. Nothing from the answer was + # applied, so the map in the request is still the current state. + def self.repair(request : Request, envelope : Ops::Envelope, issues : Hash(String, String)) : Request + answered = envelope.ops.map_with_index do |op, position| + {"op_id" => JSON::Any.new(Ops.id_for(position))}.merge(compact_op(op)) + end + text = String.build do |io| + io << "REPAIR\n" + io << "Your previous answer to this request has not been applied: the application rejected the operations listed under REJECTED, so the map is exactly as described above. " + io << "Answer again with the whole corrected proposal. Keep every operation that was not rejected as it was, fix each rejected operation or leave it out, and add nothing else.\n" + io << "PREVIOUS ANSWER summary: " << envelope.summary << '\n' if envelope.summary.presence + io << "PREVIOUS ANSWER ops: " << answered.to_json << '\n' + io << "REJECTED\n" + issues.each { |id, reason| io << id << ": " << reason << '\n' } + end + request.copy_with(parts: request.parts + [TextPart.new(text)] of Part) + end + # Earlier turns of this edit: the instruction, the ops answered and what the # person did with them. def self.history_text(history : Array(Turn)) : String diff --git a/src/placeos-rest-api/utilities/llm/runner.cr b/src/placeos-rest-api/utilities/llm/runner.cr index acd19830..a7a0d13e 100644 --- a/src/placeos-rest-api/utilities/llm/runner.cr +++ b/src/placeos-rest-api/utilities/llm/runner.cr @@ -4,7 +4,8 @@ require "placeos-models/upload" module PlaceOS::Api::Llm # The body of the spawned fiber for a map edit: the vendor call under the # slot the request handler reserved, made once more at a larger output - # budget when the first answer was cut off. + # budget when the first answer was cut off, and once more with the reasons + # when the server rejects some of the answer's operations. # # Everything it needs is captured before `spawn`: inside the fiber there is # no request and `Log.context` is empty. The context the editor posted is @@ -60,11 +61,13 @@ module PlaceOS::Api::Llm reasoning_effort: context.adapter.default_effort, )) - answer = context.adapter.complete(request) + asked = request + answer = context.adapter.complete(asked) results << answer if answer.finish == "length" && !cancelled?(context.job_id) - answer = context.adapter.complete(request.copy_with(max_output_tokens: MAP_AI_RETRY_OUTPUT_TOKENS)) + asked = request.copy_with(max_output_tokens: MAP_AI_RETRY_OUTPUT_TOKENS) + answer = context.adapter.complete(asked) results << answer end @@ -75,6 +78,11 @@ module PlaceOS::Api::Llm envelope = Ops.parse(answer.text) checked = Ops.check(envelope.ops, context.context) + repair = nil + if MAP_AI_REPAIR && !checked.issues.empty? && !cancelled?(context.job_id) + envelope, checked, repair = repaired(context, asked, envelope, checked, results) + end + issues = {} of String => JSON::Any checked.issues.each { |id, reason| issues[id] = JSON::Any.new(reason) } @@ -86,6 +94,7 @@ module PlaceOS::Api::Llm "questions" => JSON::Any.new(envelope.questions.map { |question| JSON::Any.new(question) }), "usage" => JSON::Any.new(usage_of(results, context.model)), } + written["repair"] = JSON::Any.new(repair) if repair rescue ex Log.warn(exception: ex) { "map AI vendor call failed" } failure = ex @@ -104,6 +113,45 @@ module PlaceOS::Api::Llm give_back(held) end + # The repair round: `request` again with the rejected ops and why. Its + # answer replaces the first only when it is whole, parses, keeps at least + # as many ops the server accepts, and has fewer rejected. Any failure of + # this call keeps the first answer. + private def self.repaired( + context : Context, + request : Request, + envelope : Ops::Envelope, + checked : Ops::Checked, + results : Array(Result), + ) : Tuple(Ops::Envelope, Ops::Checked, Hash(String, JSON::Any)) + rejected = checked.issues.size + accepted = envelope.ops.size - rejected + remaining = nil + begin + answer = context.adapter.complete(Prompt.repair(request, envelope, checked.issues)) + results << answer + if answer.finish != "length" + candidate = Ops.parse(answer.text) + rechecked = Ops.check(candidate.ops, context.context) + remaining = rechecked.issues.size + if remaining < rejected && candidate.ops.size - remaining >= accepted + return {candidate, rechecked, repair_note(rejected, remaining, true)} + end + end + rescue ex + Log.warn(exception: ex) { "map AI repair call failed, keeping the first answer" } + end + {envelope, checked, repair_note(rejected, remaining, false)} + end + + private def self.repair_note(rejected : Int32, remaining : Int32?, used : Bool) : Hash(String, JSON::Any) + { + "rejected" => JSON::Any.new(rejected.to_i64), + "remaining" => JSON::Any.new(remaining.try(&.to_i64)), + "used" => JSON::Any.new(used), + } + end + # Releases the run's slot if it still holds it; returns false, the new state. private def self.give_back(held : Bool?) : Bool Llm.slots.release if held From f1c3adfa3d7336f1d52d16cac43c15b925103c2e Mon Sep 17 00:00:00 2001 From: Cameron Reeves Date: Wed, 21 Oct 2026 12:57:54 +1100 Subject: [PATCH 3/3] style(map-ai): rename a local that shadowed the reorder block's (PPT-2832) Co-Authored-By: Claude Opus 5.5 --- src/placeos-rest-api/utilities/llm/ops.cr | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/placeos-rest-api/utilities/llm/ops.cr b/src/placeos-rest-api/utilities/llm/ops.cr index e4629054..13d9b41d 100644 --- a/src/placeos-rest-api/utilities/llm/ops.cr +++ b/src/placeos-rest-api/utilities/llm/ops.cr @@ -341,8 +341,8 @@ module PlaceOS::Api::Llm if present?(op["after"]?) after, issue = index.resolve(op["after"]) return "after: #{issue}" if issue - if after && (sibling = index.sibling_issue(parent, after)) - return sibling + if after && (misplaced = index.sibling_issue(parent, after)) + return misplaced end end geometry = Schema::SHAPE_GEOMETRY_ATTRIBUTES[shape]