Conversation
The single default-config key creation form (and the context, override, and variant forms) could not create a key when the type schema had no top-level `type` field — e.g. a JSON Schema combinator like `anyOf`/`oneOf`/`allOf`/ `$ref`, or a bare `const`. `SchemaType::try_from` errored with "type not defined in schema", and the Default Value input was rendered disabled, so no value could be entered and the key could not be saved. The `SchemaType` is only used to pick which input widget to render; the value itself is validated against the full JSON Schema on the backend. So a missing top-level `type` should not be a hard failure. Add a `SchemaType::Any` variant, returned by `try_from` when the schema object has no `type`. It maps to a free-form Monaco/JSON editor and parses input as arbitrary JSON. Genuinely malformed schemas (present-but-invalid `type`) still error as before. This fixes every form that consumes `SchemaType` in one place; the match arms are compiler-enforced. Verified with unit tests (schema resolution, InputType mapping, JSON parsing) and end-to-end in the running app: an `anyOf` key now renders an editable JSON value editor and persists via POST /default-config. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Review changes with Changed Files
|
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (4)
Included review availability: Your plan provides up to 2 included reviews per hour; 1 remains after this review. WalkthroughThe frontend now represents schemas without a top-level type as ChangesAny schema support
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix Suggested reviewers: Merge Risk: ⚪ Minimal · up to Untyped schemas now receive a JSON editor and valid JSON handling without an identified regression in the reviewed paths. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. A rabbit hops where schemas grow Comment |
Collapse the new `view!` block to a single line to satisfy `leptosfmt --check` (run by `make check` in CI). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
| /// Schema has no resolvable top-level `type` (e.g. `anyOf`/`oneOf`/`allOf`/`$ref`). | ||
| /// The value is accepted as free-form JSON and validated against the full schema | ||
| /// on the backend. |
There was a problem hiding this comment.
can we please remove the inline comments in both the places
There was a problem hiding this comment.
Done — removed both the doc-comment on the Any variant and the comment in try_from (commit 968ac81).
| SchemaType::Single(JsonSchemaType::Null) => { | ||
| Value::String(String::from("null")) | ||
| } | ||
| SchemaType::Any => Value::Null, |
There was a problem hiding this comment.
did we test this ?
I think this does not work correctly for some reason
that's why we use
Value::String(String::default()) in other places and Value::String(String::from("null")) for Null
so I think we should use either of these only over here as well
There was a problem hiding this comment.
Good catch on the default. It does render/edit correctly (verified in the browser — the Default Value became an editable Monaco editor showing null), and the other Monaco cases actually use real JSON values (Object -> {}, Array -> []), so Value::Null wasn't broken per se. But you're right that it's a poor default: null fails validation for basically every real anyOf/oneOf/const schema, so the user hits a validation error unless they edit it. Changed it to Value::String(String::default()) (commit 968ac81) — a valid editable default for the common anyOf: [string, ...] case and consistent with the Multiple convention. Skipped "null" since that seeds the literal string.
…r Any
Per review feedback on the SchemaType::Any support:
- Remove the explanatory comments on the `Any` variant and the `try_from`
early-return.
- Default value for `Any` is now `Value::String(String::default())` instead of
`Value::Null`. `null` fails validation for typical anyOf/oneOf/const schemas,
so an empty string is a safer editable default and matches the `Multiple`
("unknown type") convention.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
| SchemaType::Single(JsonSchemaType::Null) => { | ||
| Value::String(String::from("null")) | ||
| } | ||
| SchemaType::Any => Value::String(String::default()), |
There was a problem hiding this comment.
Any doesn't necessarily mean string is a valid type. For example, {"anyOf":[{"type":"number"},{"type":"boolean"}]} would resolve to Any, but "" would be an invalid default. Should Any avoid providing a default value altogether (e.g. Option) and let the Monaco input/user provide the value?
Not a blocker for this PR since backend schema validation still protects correctness. Good to merge; we can handle this as a follow-up.
This branch has not been deployed
3 participants
Problem
Creating a Default Config key (the single key creation UI) was impossible when the type schema had no top-level
typefield — e.g. a JSON Schema combinator (anyOf/oneOf/allOf/$ref) or a bareconst.Example schema from the report:
{ "anyOf": [ { "type": "string" }, { "const": "default_str_ignore_this" } ] }SchemaType::try_fromerrored withtype not defined in schema, and the Default Value input was rendereddisabled, so no value could be entered and the key could not be saved. The same gap affected the context, override, and variant forms and the default-config detail page.Root cause
SchemaType::try_from(crates/frontend/src/schema.rs) hard-required a top-leveltype. ThatSchemaTypeis only used to choose which input widget to render — the value itself is validated against the full JSON Schema on the backend (jsonschema). So a missing top-leveltypeshould not be a hard failure.Fix
Add a
SchemaType::Anyvariant, returned bytry_fromwhen the schema object has notype. It:InputType::Monaco)serde_json::from_str::<Value>)Genuinely malformed schemas (present-but-invalid
type, e.g."type": "foobar") still error as before. Because the match arms onSchemaTypeare exhaustive/compiler-enforced, this fixes every form that consumes it in one place.Files
crates/frontend/src/schema.rs— newAnyvariant;try_fromreturnsOk(Any)on missingtype;default_valuearm; unit testscrates/frontend/src/components/input.rs—InputType::from→ Monaco forAny;parse_inputparses free-form JSON; unit testscrates/frontend/src/components/override_form.rs—TypeBadgerenders ananybadgecrates/frontend/src/components/cohort_schema.rs—Anyexcluded from string/number-array cohort filtersNote on UX
With this schema the Default Value is now a free-form JSON editor: values are entered as JSON (a string must be quoted, e.g.
"default_str_ignore_this"). Unquoted text shows anot a valid JSON valuehint; the backend validates against the full schema on submit.Verification
cargo test -p frontend --lib— 6 new unit tests pass (schema resolution,Any→Monaco mapping, JSON parsing, invalid-type still errors)cargo check -p frontend --libon host and--target wasm32-unknown-unknowncompile clean;cargo clippycleananyOfkey now renders an editable JSON value editor (no error), and creation persists viaPOST /default-config → 200🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes