Skip to content

forms: draw a schema-stated closed set as a picker, and gate on it - #395

Merged
Yaraslaut merged 1 commit into
masterfrom
fix/386-forms-enum-choice
Sep 2, 2026
Merged

forms: draw a schema-stated closed set as a picker, and gate on it#395
Yaraslaut merged 1 commit into
masterfrom
fix/386-forms-enum-choice

Conversation

@Yaraslaut

Copy link
Copy Markdown
Member

Closes #386.

schemaJson<A>() describes a glz::enumerated enum class member completely — glaze emits a closed oneOf of const alternatives, each carrying its own title — but DynamicForm read none of it. A three-value set drew a TextField, and ready was true for role = "Emperor".

Two independent halves

resolveProp discarded the alternatives. It collapsed any oneOf/anyOf to its first non-null branch. That path was added for the nullable-$ref shape (#189), where the branches differ only in nullability and collapsing is right. Applied to a closed set, whose branches differ in value, it dropped every alternative but the first — and merged that branch's const onto the field as if the schema had pinned the value.

fields never looked at const/enum. isChoice keyed solely on x-optionsAction, so an enum member fell through to the text field and fieldJsonLiteral returned whatever was typed.

The fix

enumChoices() recognises "every branch bar {"type":"null"} carries a const" as a closed set — distinct from the nullability shape, where one branch has no const and the property falls back, so #189's case still collapses as before. The bare JSON-Schema enum keyword is read the same way. The set draws with the combo box Choice already uses (no options action, no fetch — values and labels are both already in the schema), taking the field_ objectName from the hidden TextField exactly as the boolean's CheckBox does. fieldJsonLiteral refuses an out-of-set value, so the gate means what it says, and resetFields clears a combo by currentIndex because it has no writable text.

Two shipped rungs were rendering free-text boxes for fully enumerated members today: pastebin::CreatePaste's visibility/editability, and bookmarks::CreateBookmark's visibility.

Spec/code disagreement, resolved in the spec's favour

docs/spec/forms/forms.md:819 claimed glaze does not emit oneOf. It does, and that sentence was the stated rationale for treating oneOf as nullability-only — so it was load-bearing, not cosmetic. The spec gains a "Closed sets" section and loses the false claim.

The issue's framing was also slightly too broad: the oneOf-of-consts shape appears only for an enum declaring a glz::meta/glz::enumerate. That was measured directly from a schemaJson<>() dump and is now reflected in both the spec and the code comments. An enum without a glz::meta gets a different, worse treatment — filed separately as #392 rather than folded in here.

Verification

src/qt/forms/tests/tst_DynamicFormEnumChoice.qml, 17 cases. Every schema in it is pasted verbatim from a real schemaJson<>() dump (kanban SetMemberRole, pastebin CreatePaste) rather than hand-written to match the implementation.

Non-vacuity, measured — DynamicForm.qml reverted to its parent commit and rebuilt, new test left in place:

enum PASS: 3   enum FAIL: 16
FAIL!  : ...test_an_enum_renders_a_selection_control_not_a_text_field() 'verify()' returned FALSE
FAIL!  : ...test_a_value_outside_the_set_leaves_the_form_not_ready() Compared values are not the same
FAIL!  : ...test_choosing_an_option_serialises_the_enum_name_glaze_reads() Uncaught exception: Cannot assign to non-existent property "currentIndex"

16 of 19 entries fail; the 3 that pass are initTestCase, cleanupTestCase and one preservation guard. With the fix, 19/19 pass. One case originally passed against the unfixed code — it read const off the field descriptor, which never carried it — so it was rewritten to assert on resolveProp's own output and re-measured. The resetFields guard was reverted alone as well: Uncaught exception: Cannot assign to non-existent property "text".

Regressions: ladder_pastebin_tests 769 assertions, ladder_bookmarks_tests 837, ladder_kanban_tests 1240 — all pass.

Not verified: the HTML renderer (morph::render). This change is confined to the Qt/QML renderer; whether the HTML one has the same gap is untested either way.

On the QML suite total: it reports 250 passed, 6 failed. The 6 are module "Lightweight.Migrations" is not installed, from _deps/lightweight-src/src/tools/dbtool-gui/tests/qml/ — a vendored dependency's own QML tests that the harness scans. They fail identically before this change and are unrelated to it.

🤖 Generated with Claude Code

@codecov

codecov Bot commented Sep 2, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

)

`schemaJson<A>()` describes a `glz::enumerate`d C++ `enum class` completely:
glaze emits a `oneOf` of `const` alternatives, each with its own `title`. The
shipped Qt/QML renderer read none of it.

Root cause, two halves that compound:

  * `resolveProp` collapsed any `oneOf`/`anyOf` to its first non-null branch.
    That path was added for the nullable-`$ref` shape (#189), where the
    branches differ only in *nullability*; applied to a closed set, whose
    branches differ in *value*, it discarded every alternative but the first
    and left that one's `const` merged onto the field as though the schema had
    pinned it.
  * `fields` then never looked at `const` or `enum` at all -- `isChoice` was
    set solely by `x-optionsAction` -- so the member fell through to the plain
    `TextField`, and `fieldJsonLiteral` returned whatever was typed. The submit
    gate reported `ready` for `role = "Emperor"` and assembled a body for it,
    which is the opposite of what a client-side gate exists to say.

`enumChoices()` now recognises "every branch bar `{"type":"null"}` carries a
`const`" as a distinct shape from the nullability one -- one branch without a
`const` and the whole property falls back, so #189's shape still collapses to
its typed branch. The bare JSON-Schema `enum` keyword is read the same way. The
resulting `{label, valueJson}` rows have the same shape a fetched `Choice`'s
do, so they feed the combo box `isChoice` already draws, with no options action
and no round trip; the enum's combo claims the `field_` objectName from the
plain `TextField`, exactly as the boolean's `CheckBox` does. `fieldJsonLiteral`
refuses a value outside the set, because here -- unlike a `Choice`, whose
option list is a snapshot that may be stale -- membership is decidable on the
client. `resetFields` clears a combo by `currentIndex`, since it has no
writable `text`.

Two shipped rungs were drawing free-text boxes for fully enumerated members
today: `pastebin::CreatePaste`'s `visibility`/`editability` and
`bookmarks::CreateBookmark`'s `visibility`.

`docs/spec/forms/forms.md` gains a "Closed sets" section and loses the claim
that glaze does not emit `oneOf` -- which was the stated reason `oneOf` was
treated as nullability-only, and was false.

`tst_DynamicFormEnumChoice.qml` drives every schema verbatim from
`schemaJson<>()` for a shipped action. Reverting this change fails 15 of its
17 cases.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@Yaraslaut
Yaraslaut force-pushed the fix/386-forms-enum-choice branch from f615ebf to 3abd73b Compare September 2, 2026 15:44
@Yaraslaut
Yaraslaut merged commit e675253 into master Sep 2, 2026
6 checks passed
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.

forms: DynamicForm renders a C++ enum class as a free-text field, and its gate accepts values outside the set

1 participant