feat(signage-ai): image generation runtime and controllers (PPT-2741) - #447
Open
camreeves wants to merge 12 commits into
Open
feat(signage-ai): image generation runtime and controllers (PPT-2741)#447camreeves wants to merge 12 commits into
camreeves wants to merge 12 commits into
Conversation
Generate and edit signage artwork through a domain's own vendor account. A request validates, reserves a slot per vendor call, writes a job row and hands off to a fiber, answering 202. The client long polls jobs/:id, which holds until the version moves or the wait runs out, so a candidate reaches the browser about half a second after it lands without needing a socket. - ImageGen::Adapter with OpenAI (and Azure OpenAI, same wire shape) and Gemini on Vertex. Vertex only for Google: an AI Studio key carries neither the indemnity nor the no training terms. - Slots caps concurrent vendor calls per replica. A request that cannot reserve every call it needs is told the service is busy and no row is written, so there is never a queued job nobody is working on. - Store writes results through the same Storage and Upload machinery the uploads controller uses, from outside a request. - Prompt asks for a clear headline area and no lettering, because the words and the logo are composited by the browser. - Sweep removes candidates nobody kept after the retention window and fails jobs left running by a replica that went away. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
28 specs across the two controllers: capabilities on and off, the three group permission refusals, quota, the idempotency replay, a full run with the vendor and the storage PUT mocked through to the Upload row, a vendor refusal landing on the job as a moderation failure, both long poll behaviours, cancel, claim, usage, and that credentials never appear in a provider response. Also fixes a slot leak the specs turned up: the runner returned without handing back its reserved slots when the job row had gone, so a replica lost capacity until it restarted. Slots are now tracked in an atomic ledger that every exit path drains exactly once. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Turning off the text layer means the model produces the finished poster, but the prompt still asked for a background and never mentioned the wording, so the words never appeared. It now asks for a poster rather than a background, and for the brief's wording to be rendered legibly. Found running the flow in a browser with the toggle off. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A field left out of the body is left alone, but a field sent empty now clears the stored value. Without the second half an endpoint could be set and never unset, and an empty endpoint is exactly what sends a provider to the vendor's own host rather than to a gateway: a row pointed at a test endpoint could not be moved to the real one through the API. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…aper credentials check Image models left alone produce a recognisable house style: centred title over a purple gradient, glowing orbs, floating geometry. Every request now carries a counter-brief ahead of the brand kit and the brief. Most of it applies whatever draws the words. Four lines do not: when the app composites the headline afterwards the model is producing a background, so art directing typography, asking it not to place text over a background, and telling it to reproduce supplied wording all argue with the layout instruction that follows. Those are held back in that mode and specced both ways. Also fixes the credentials check, which asked for 2048x2048 at the medium tier: a full price image taking half a minute, for something documented as one small image. It now asks for 1024x1024 at the vendor's cheapest tier, which needed an explicit size override and a draft quality that maps to OpenAI's low. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Editing changed far more than was asked. Three causes, all ours rather than the model's: - A first edit sent the user's words as a *brief*, not an instruction, so the "keep everything else" framing was dropped entirely. It only appeared from the second refine onward. - The generation art direction went out with every edit, including "establish a specific visual idea and let that idea determine the typography, image treatment, composition and colour palette". That is a redesign brief, and it explains the changed fonts and colours exactly. - The layout line went out too, so an edit of a text heavy poster was also told it was making a background and to render no text at all. An edit now gets its own prompt: preservation first, the original brief only as context, each earlier change listed as already applied, and the new one named as the only change to make. None of the generation direction is sent. gpt-image-2 still regenerates the whole frame rather than painting into a region, so this reduces drift rather than removing it. If it is still too loose the fallback is to route edits to Nano Banana 2, which measures best of the current models at preserving unedited regions. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
An edit came back at the size the aspect dropdown implied, so a 2:3 poster edited as 9:16 was reframed. The aspect describes the screen the artwork is destined for, which is not the shape of the thing handed to us, and an edit must not change the shape at all. The runner already reads the source's dimensions out of the file header, so it now derives the output size from those, snapped to what gpt-image-2 will accept: both edges a multiple of 16, a long edge within 3840, and the total between 655,360 and 8,294,400 pixels. Sources outside the vendor's 3:1 limit fall back to the aspect table. Snapping to 16 moves the area either way and both bounds are hard, so it steps back inside them; a spec caught 8000x6000 landing 12k pixels over the ceiling. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…hich Three things were wrong with references. They never reached the vendor on a generate. The JSON generations call carries a prompt and nothing else, so anything attached was fetched, counted and dropped. A generate that has references now goes through the multipart endpoint, which is how the vendor takes input images. The model was not told what "image 2" meant. It is handed a list with no names, so a brief naming one was pointing at nothing. The prompt now states how many are attached and in what order, and on an edit says that the first image is the one being edited so the person's first reference is still image 1. The logo was being appended to that same list. It was harmless while generates dropped their images and would not be now: the app composites the real logo afterwards, so an attached one is a logo the model draws as well, leaving two. It goes only on an edit now, which is what already happened in practice. Uploads attached to a request are tagged so the existing sweep can clear them, for the times the browser does not get to.
Not every poster is meant to look like the company. `use_branding` defaults to true and keeps today's behaviour; false leaves the organisation, palette and tone out of the prompt, and is recorded on the job so what a poster was asked for stays readable afterwards. The never-include list is not part of the switch. It says what the organisation will not have on a screen, which holds whether or not the poster is wearing its colours, and turning off "use our colours" is not consent to competitor logos.
Provider rows are scoped to the calling domain. `index` returned every row in the deployment, and every other route took a bare id, so an administrator of one customer could read, change, delete and spend against another's provider. The shared fallback row stays readable everywhere and writable nowhere. A reference upload is only tagged for the sweep when this caller made it in the last fifteen minutes. Tagging any untagged upload marked a file somebody had attached from their library for deletion, dated from the file's own age, so one older than the retention window was eligible immediately. Source and reference images are bounded. The read is streamed and capped rather than buffered and measured afterwards, and an upload is rejected on its recorded size and file type before anything fetches it. The brand logo goes through the size and type check but not the ownership ladder: it is a domain asset nobody personally owns, and putting it through `readable_upload` dropped it from every edit a customer made while still advertising the toggle. `claim` no longer requires the item's file to be the candidate itself. Drawing words over the artwork saves a flattened copy, so the ids never matched and the job to item link was never written for the posters people actually make. Five sites did `version = version + 1` then `save`, which can move the version backwards and strand a long polling client. All of them bump in SQL now. Also: a concurrent duplicate submission replays the job it lost to instead of answering with an empty 422, the Google token call cannot hold a slot forever, the provider test takes a slot like any other vendor call, and the claim scan no longer raises on a job with a candidate that never landed. Specs for the two paths nothing covered: a customer with Create on a signage group generating an image, and the brand logo reaching the vendor for a caller who does not own it.
… specs The per-image ceiling still allowed eight attachments at once, so one request could pull 160MB into the process. Capped in aggregate. Three specs for the headline blocker of the last commit, which had none: another domain's provider is absent from the list and 404s on show, update, delete and test; the shared row reads but does not write; and a create lands in the caller's domain whatever the body asks for.
CI runs crystal-style on every push and the spec file was not formatted.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part of the AI signage feature (PPT-2741). Depends on PlaceOS/models#326.
What this adds
The runtime that talks to image vendors, and the two controllers in front of it.
POST /signage/ai/generatetakes a brief and returns a job.POST /signage/ai/edittakes an existing image and a change. Both run asynchronously against a slot ledger so one domain cannot occupy every worker.POST /signage/ai/:id/claimpromotes a chosen candidate into the media library as a normal upload.There are three adapters behind a common interface: OpenAI images, Azure OpenAI, and Google Vertex. Adding a fourth means one file.
signage/ai/providersis the admin side: list, create, update, delete, and a test endpoint that does one small generation and throws the result away, so you find out the credentials are wrong when you save them rather than when a user tries to make a poster.Notes for review
The prompt builder is worth a look. It composes the user's brief with the organisation's brand kit, and a request can opt out. An edit is deliberately sent differently to a generation: an edit that reads as a redesign brief gets you a new poster instead of a changed one, which was a real bug during development.
Two things that came out of a pre-push audit and are worth understanding rather than skimming:
Permission checks.
check_create_permissionrefuses a non-support caller that does not supply agroup_id. The browser was not sending one, so for a while no customer could generate anything and only my admin account worked. Fixed, with the positive spec that was missing.Brand logo reads. Fetching the organisation's logo goes through
vendor_readable?, which checks size and type only. It deliberately does not go throughreadable_upload, which is a caller-ownership gate. Routing it through the ownership gate madeinclude_logoa silent no-op for every customer, which is the sort of thing that passes review because it looks safer.Request size is capped by streaming with a bounded
IO.copyrather than checking after the body is already in memory. The earlier version checked the size afterHTTP::Client#gethad buffered the whole thing, so it was not a ceiling at all.Merge order
After PlaceOS/models#326 is merged and tagged. The migration has to run before this code serves traffic, since the endpoints query tables that will not exist yet.
Testing
Controller specs for both endpoint groups, prompt composition, size limits, and the slot ledger.
crystal tool formatis clean. Compiles against the models branch.