Skip to content

feat(formdesigner): create forms without an LLM via POST /forms - #1070

Draft
jelitox wants to merge 1 commit into
devfrom
feat/formdesigner-no-llm-form-creation
Draft

feat(formdesigner): create forms without an LLM via POST /forms#1070
jelitox wants to merge 1 commit into
devfrom
feat/formdesigner-no-llm-form-creation

Conversation

@jelitox

@jelitox jelitox commented Jul 30, 2026

Copy link
Copy Markdown
Collaborator

Problem

POST /api/v1/forms was LLM-only: it required a prompt in the body and
returned 503 No LLM client configured for form creation without a client.

That left a form-builder UI with no way to get a blank canvas to work on.
PUT /forms/{id}, PATCH /forms/{id} and PATCH /forms/{id}/operations all
404 on an unknown form_id, so the only LLM-free ways to bring a form into
existence were POST /forms/from-db (NetworkNinja definition) or cloning an
existing one. "New form → blank → drag controls" was not expressible.

Change

POST /api/v1/forms is now dual-mode, selected by the presence of the
prompt key:

Body Path Response
{"prompt": "..."} CreateFormTool (LLM) — unchanged 200 {form_id, title, url}
anything else, incl. {} _create_blank_form — no LLM 201 + full FormSchema
# 1. "New" → blank form (an empty body is enough)
curl -X POST /api/v1/forms -d '{}'
# → 201 {"form_id":"untitled-form","version":"1.0",
#        "sections":[{"section_id":"section_1","fields":[]}], ...}

# 2. Control catalog for the toolbar
curl /api/v1/form-controls

# 3. Drop controls on the canvas, one at a time or batched
curl -X PATCH /api/v1/forms/untitled-form/operations \
  -H 'If-Match: 1.0' \
  -d '{"operations":[{"op":"add_field","section_id":"section_1",
        "field":{"field_id":"full_name","field_type":"text","label":"Full Name"}}]}'

Design decisions

  • Seeded section. A blank form carries one empty section section_1,
    because add_field requires an existing section_id — without it the canvas
    would need an add_section round trip first. Pass "sections": [] for none.
  • form_id. Explicit when supplied and matching FORM_ID_RE
    (400 on spaces, /, .., control characters; 409 when taken). Otherwise
    derived from title, with a random hex suffix on collision — so clicking
    "New" repeatedly never fails with a 409.
  • Handler-controlled keys. version is always "1.0",
    published_version always None (publishing stays POST /forms/{id}/publish),
    and tenant always comes from the session, never the body — a caller cannot
    write into another tenant. Every other FormSchema field passes through.
  • Mode by key presence, not truthiness. {"prompt": ""} keeps its
    historical 400 prompt is required rather than silently creating a blank form.

Storage-aware duplicate detection

FormRegistry.get() is memory-only, the in-memory cache is hydrated per
tenant
, and PostgresFormStorage.save() upserts (ON CONFLICT ... DO UPDATE).
A memory-only existence check would therefore let a "new" form silently
overwrite a persisted form belonging to a tenant that has not been hydrated
yet — data loss.

This PR adds the opt-in FormRegistry.contains(..., include_storage=True),
which probes storage when the form is not in memory and treats a storage error
as "cannot prove absence" (refusing to create beats overwriting).

It also verifies object identity after register(overwrite=False): that call is
a silent no-op on a taken id, so the loser of a concurrent create would
otherwise receive a 201 describing a form that was never stored, and its
subsequent /operations calls would edit the winner's form.

Testing

  • 35 new tests in tests/unit/api/test_create_blank_form.py, including the full
    round trip (create blank → add 2 fields + 1 section via /operations → assert
    persistence), a mocked storage backend for the upsert-overwrite case, and a
    monkeypatch-simulated lost race.
  • Unit suite: 1262 passed / 14 failed. The 14 failures are pre-existing —
    verified against the same base with the change stashed (1227 passed, same 14).
    They are stale assertions unrelated to this change, e.g.
    test_form_controls_endpoint does not know about the
    supported_effects/supported_operations/supported_operators keys that
    TASK-1529 added on 2026-06-12, and several count FieldType against an
    outdated total.
  • ruff check clean on all touched files.

Reviewed against an independent adversarial pass

An independent review raised six findings. Four are fixed here (the
storage-overwrite bug, the create race, prompt truthiness, and a $-vs-\Z
regex anchor that let a trailing newline through). Two are real but
pre-existing and out of scope, flagged for a follow-up decision:

  1. PATCH /forms/{id}/operations does not pass through RBAC.
    handle_operations never calls _rbac_shadow_gate, while PUT/PATCH
    enforce update_form/patch_form. Pre-existing since FEAT-152 and currently
    moot (RBAC defaults to shadow mode, log-only), but this PR makes
    /operations the recommended builder path, so the gap now matters more.
    Fixing it is not mechanical: handle_operations is a module-level function
    with no handler instance.
  2. A persistence failure still returns 2xx. FormRegistry.register()
    catches storage exceptions, logs, and returns normally — so a 201 does not
    prove the form survived a restart. This is shared with PUT/PATCH/
    /operations; changing it alters register()'s contract for every caller,
    so this PR stays consistent with them rather than diverging.

Notes

No migration and no breaking change: the prompt path keeps its exact status
codes and response shape, and contains() gains a defaulted keyword argument.

Developed with Spec Driven Development

`POST /api/v1/forms` was LLM-only: it required a `prompt` and returned
503 without a client. A form builder therefore had no way to get a blank
canvas to work on — PUT/PATCH/operations all 404 on an unknown form_id,
leaving only `from-db` or `clone` of an existing form.

Make the endpoint dual-mode, selected by the presence of the `prompt`
key:

- with `prompt` — unchanged CreateFormTool path (503 without a client,
  400 on an empty prompt, response `{form_id, title, url}`).
- without `prompt` — new `_create_blank_form`: builds the FormSchema
  straight from the body with no LLM, returns 201 with the full schema.
  An empty body is valid, so a "New form" button can POST `{}` and then
  add controls with `PATCH /forms/{form_id}/operations`.

Blank forms are seeded with one empty section (`section_1`) so
`add_field` has a target; `"sections": []` opts out. `form_id` is taken
from the body when it matches FORM_ID_RE (409 when taken) or derived
from `title` with a random suffix on collision, so clicking "New"
repeatedly never fails. `version`, `published_version` and `tenant` are
handler-controlled and ignored from the body — tenant always comes from
the session.

Duplicate detection has to consult storage, not just memory:
`FormRegistry.get()` is memory-only, the cache is hydrated per tenant,
and `PostgresFormStorage.save()` upserts, so a memory-only check would
let a "new" form silently overwrite a persisted one belonging to a
non-hydrated tenant. Adds the opt-in
`FormRegistry.contains(..., include_storage=True)` for that, treating a
storage error as "cannot prove absence". Also verifies object identity
after `register(overwrite=False)` — it is a silent no-op on a taken id,
so the loser of a race would otherwise get a 201 for a form that was
never stored.

Developed By: Javier León (With SDD) <jleon@trocglobal.com>
@jelitox

jelitox commented Jul 30, 2026

Copy link
Copy Markdown
Collaborator Author

Paused pending design decisions

Holding this PR while two things land: FEAT-388 (sdd/specs/deterministic-creationformtool.spec.md, approved, no task index yet) and the in-flight brainstorm on switching form URLs from the form_id slug to a stable UUID. Both overlap this change and should decide its shape.

Recording what the investigation turned up, since it is input for that brainstorm.

The blank-form affordance in this PR is wrong

Review feedback, accepted: a blank canvas must not be a server-side object. Opening "New form" in Word does not write document1.docx to disk, and POST /forms {} should not write an untitled-form row. The draft belongs in the client (LocalStorage); persistence starts at "Save", with a complete payload.

That makes the seeded empty section and the empty-body affordance the parts to drop. The underlying capability — persist a complete, client-built schema with no LLM — is still needed, and FEAT-388 already specifies it at the tool layer (FormAssembler, with schema / sections / fields inputs and prompt becoming optional).

PUT /forms/{form_id} does not upsert today

Worth stating precisely, because the natural reading of PUT ("edit, and create if absent") is not what the code does — api/handlers.py:1124-1128:

existing = await self.registry.get(form_id, tenant=tenant)
if existing is None:
    return JSONResponse({"error": f"Form '{form_id}' not found"}, status=404)

PUT, PATCH and PATCH /operations all 404 on an unknown form_id. So the "canvas → name it → Save → PUT" flow is not expressible yet; it needs update_form to become a real upsert. That is a small change and probably the right one.

For completeness, no other endpoint in this package creates a form from a payload. The POST surface is /forms (LLM), /forms/from-db (DatabaseFormTool, no LLM but requires a NetworkNinja definition), /{id}/edit (LLM), /{id}/clone, /{id}/validate, /{id}/data, /{id}/partial, /{id}/events/{name}, /{id}/publish, /fields. The UI package has no design/canvas page either — ui/routes.py:103 is submit_form (filling a form, not editing its design).

The DB already generates a UUID and it is discarded

This is the strongest argument for the form_uid change, and it is worse than "unused". services/storage.py:151-162:

CREATE TABLE IF NOT EXISTS {qt} (
    id UUID PRIMARY KEY DEFAULT gen_random_uuid(),
    form_id VARCHAR(255) NOT NULL,
    version VARCHAR(50) NOT NULL DEFAULT '1.0',
    ...
    UNIQUE(form_id, version)
);
  • The id column is never read back: all three SELECTs in the module retrieve only schema_json, created_at (storage.py:181, :190, :202), and FormSchema has no form_uid field. The identity the database hands out is thrown away on every load.
  • The natural key is (form_id, version) — the slug. So a rename does not rename: it inserts a new row and orphans the old one. There is no stable identity for a form across renames anywhere in the stack, not just in the URLs.
  • _upsert_sql (storage.py:165-176) conflict-targets (form_id, version), which is also why a memory-only existence check can silently overwrite a persisted form — the bug this PR fixes with FormRegistry.contains(..., include_storage=True). That fix stands on its own regardless of what happens to the rest.

Naming

The suggestion to split /api/v1/ai_forms (LLM) from /api/v1/forms (deterministic CRUD) is mechanically cheap — it is route registration in api/routes.py — but breaking for existing consumers, so it belongs in the same decision as the form_uid change rather than here.

Developed with Spec Driven Development

@jelitox
jelitox marked this pull request as draft July 30, 2026 12:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant