fix(ui): resolve metadata agent form crash and manifest JSON editor UX#30141
Open
aniketkatkar97 wants to merge 10 commits into
Open
fix(ui): resolve metadata agent form crash and manifest JSON editor UX#30141aniketkatkar97 wants to merge 10 commits into
aniketkatkar97 wants to merge 10 commits into
Conversation
The add/edit metadata agent form crashed with "Unsupported widget definition: object" for Storage (S3) services. IngestionWorkflowForm registered its RJSF widgets as raw React.lazy objects; @rjsf/utils getWidget accepts only function/forwardRef/memo, so resolving the manifestJson widget threw. Wrap the lazy widgets in the existing withSuspenseFallback (forwardRef) so getWidget accepts them while keeping code-splitting. Also fixes the Default Manifest JSON editor: - Full width: isWide() now treats uiFieldType 'code' fields as wide. - No caret jump: SchemaEditor gains an autoFormat prop (default true); the manifest widget disables it so the buffer is not re-indented on every keystroke. - No stale default: the sample is shown as a CodeMirror placeholder (muted --tw-color-text-placeholder, multi-line, editor flex-fills its wrapper) instead of being written as the field value. Adds a Playwright spec (render + edit/save round-trip, width, clears cleanly, caret stability) and unit tests for getSchemaEditorValue; updates ManifestJsonWidget tests to the placeholder contract. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
github-actions
Bot
added
safe to test
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Satisfies the space-after-comment lint rule flagged in review. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
Jest test CoverageUI tests summary
|
react-codemirror2's Controlled mode cancels CodeMirror's native change and
re-applies the value, which drops the caret that autoCloseBrackets places
between the inserted pair (typing `{` produced `{}` with the caret after `}`,
so the next character landed outside).
Add an opt-in `uncontrolled` mode to SchemaEditor that renders the UnControlled
CodeMirror so the editor owns its buffer and caret. The value prop is used only
as initial content; external updates (e.g. an async-loaded saved config) are
synced imperatively while the editor is blurred so active typing is never
disturbed. ManifestJsonWidget opts in.
Adds a Playwright assertion that typing `{` then `x` yields `{x}`.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
![gitar-bot[bot]](https://avatars.githubusercontent.com/in/827041?s=60&v=4){kind=small}
Address review edge case: an external value arriving while the uncontrolled editor is focused was skipped by the blurred-only sync and never re-applied. Track the latest value and apply it on the next blur. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
🔴 Playwright Results — 1 failure(s), 24 flaky✅ 4548 passed · ❌ 1 failed · 🟡 24 flaky · ⏭️ 96 skipped
Genuine Failures (failed on all attempts)❌
|
Code Review ✅ Approved 1 resolved / 1 findingsResolves the metadata agent form crash by wrapping lazy RJSF widgets in suspense fallbacks and improves manifest JSON editor UX with stable caret handling and proper placeholder support. No issues found. ✅ 1 resolved✅ Edge Case: Uncontrolled sync skips value updates that arrive while focused
OptionsDisplay: compact → Showing less information. Comment with these commands to change the behavior for this request:
Was this helpful? React with 👍 / 👎 | Gitar |
|
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
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.
Fixes #30142
Problem
Adding or editing a metadata agent for a Storage (S3) service crashed the UI:
IngestionWorkflowFormregisters its RJSF widgets as rawReact.lazy(...)objects.@rjsf/utilsgetWidget(v5.24.13) accepts onlyfunction/forwardRef/memo/string— aReact.lazyresult is a plain object, so resolving the storagedefaultManifest → ui:widget: 'manifestJson'threw. Introduced by #28569, which switched this form's widgets toReact.lazy.Testing the (now-rendering) Default Manifest editor surfaced three more UX bugs: it was half-width, the caret jumped while typing, and clearing it reverted to the sample JSON.
Changes
Crash fix
IngestionWorkflowForm: wrap the lazy RJSF widgets in the existingwithSuspenseFallbackHOC (returns aforwardRef, whichgetWidgetaccepts) so they resolve while keeping code-splitting. Same patternMetadataAgentsWidgetalready uses.Manifest JSON editor
IngestionObjectFieldTemplate.isWide()now treatsuiFieldType: 'code'fields as full-row (generic, no hardcoded field name).SchemaEditorgains anautoFormatprop (defaulttrue, existing consumers unchanged);ManifestJsonWidgetsetsautoFormat={false}so the buffer is not re-indented viaJSON.stringifyon every keystroke.--tw-color-text-placeholder, multi-line viapre-wrap, editor flex-fills its wrapper) instead of being written as the field value. Clearing the editor now leaves it truly empty.Testing
StorageMetadataAgentForm.spec.ts): edit form renders without crashing, manifest edits save (asserted on thePATCH /services/ingestionPipelines/{id}response) and round-trip on reopen; editor is full width, clears cleanly (placeholder muted + multi-line), and the caret stays stable (no live re-format). Verified end-to-end against a local backend + Airflow.SchemaEditor.utils.test.tscoversgetSchemaEditorValue(bothautoFormatmodes + edge cases);ManifestJsonWidgettests updated to the placeholder contract; existingvalidateManifestJsoncoverage unchanged.jest→ 33 passing.🤖 Generated with Claude Code
Greptile Summary
This PR fixes the metadata-agent form crash and improves the manifest JSON editor. The main changes are:
Confidence Score: 5/5
This looks safe to merge.
Important Files Changed
Reviews (6): Last reviewed commit: "Merge branch 'main' into s3-widget-error..." | Re-trigger Greptile
Context used (3)