fix(frontend): show why the agent config failed instead of a skeleton forever - #6431
Conversation
… forever `AgentTemplateControl` is code-split behind `lazy()`, with `AgentConfigSkeleton` as its Suspense fallback and no error boundary. A chunk that fails to load, or resolves without its named export, therefore suspends indefinitely: the panel shows its loading skeleton for good, indistinguishable from a slow network, and the only trace is a console error nobody is looking at. Wraps it in a boundary that renders the failure, and names two cases the import could otherwise swallow: a rejected chunk, and one that resolves with the export missing (a circular import mid-initialisation resolves the module with holes, which React would render as `undefined` rather than reject). Found while chasing a different infinite skeleton in this panel. That one turned out to be a stalled IndexedDB read, not the chunk — but it took hours partly because this path fails the same way, and the two were indistinguishable from the outside.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (1)
Disabled knowledge base sources:
📝 SummarySummary by CodeRabbit
WalkthroughThe agent template loader validates its named export and reports load failures. A local error boundary catches render failures and displays an error message around the suspended agent-template control. ChangesAgent template error handling
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to The PR makes the configuration failure visible instead of leaving users on an indefinite loading skeleton. No actionable merge-blocking risk remains after normal checks and review. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Full details: Docstring CoverageExplanation No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1 files. ✨ Finishing Touches📝 Generate docstrings
🧪 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Team
Run ID: 2ddbd812-1714-445a-b22f-1960e38b5f9c
📒 Files selected for processing (1)
web/packages/agenta-entity-ui/src/DrillInView/SchemaControls/SchemaPropertyRenderer.tsx
Included review availability: Your plan provides up to 8 included reviews per hour; 1 remains after this review.
| // Resolved, but the named export is missing (a circular import that has not | ||
| // finished initialising resolves the module with holes). React would then render | ||
| // `undefined` rather than reject, so name it here instead of failing opaquely. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Keep each new source comment to one short line.
The added comments at Lines 42-44, 54-55, 61-67, and 491-494 span multiple lines. Replace each block comment with one concise line or remove comments that only restate the code.
Proposed cleanup
- // Resolved, but the named export is missing (a circular import that has not
- // finished initialising resolves the module with holes). React would then render
- // `undefined` rather than reject, so name it here instead of failing opaquely.
+ // Validate the named export before rendering.
- // A rejected lazy import leaves the Suspense fallback up forever when nothing
- // catches it, which reads as "the config panel never loads" with no error anywhere.
+ // Log and rethrow lazy-load failures.
-/**
- * Renders WHY the agent config did not load, instead of leaving its skeleton up forever.
- *
- * A `lazy()` import that rejects suspends until something catches it. With no boundary here the
- * panel sat on `AgentConfigSkeleton` indefinitely, indistinguishable from a slow network, and the
- * only trace was a console error nobody was looking at.
- */
+// Show agent-template failures instead of an indefinite skeleton.
- // Render the whole agent config (instructions, model, tools, runtime) as one
- // composite control that reuses the model selector, tool picker, and enums.
- // The Suspense fallback is the SAME skeleton the schema-loading gate shows,
- // so the two gates read as one continuous frame while the chunk loads.
+ // Render the agent configuration with the shared loading skeleton.As per coding guidelines: “Hard rule. At most ONE short line per comment.”
Also applies to: 54-55, 61-67, 491-494
Source: Coding guidelines
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
Railway Preview Environment
|
2 participants
Context
The agent's Configuration panel can sit on its loading skeleton forever with no error anywhere, when the chunk behind it fails to load.
AgentTemplateControlis the heaviest control in the registry, so it is code-split:There is no error boundary on it. A
lazy()import that rejects suspends until something catches it, so the panel holdsAgentConfigSkeletonindefinitely, indistinguishable from a slow network. The only trace is a console error nobody is looking at.Found while chasing a different infinite skeleton in this same panel (#6424, a stalled IndexedDB read). That one was not the chunk, but the two fail identically from the outside, and telling them apart cost real time.
Changes
Wraps the control in a boundary that renders the failure instead of the skeleton, and names two cases the import could otherwise swallow:
m.AgentTemplateControlisundefined. React would render that asundefinedrather than reject, so it is turned into a named error listing the keys the module did expose.Tests
@agenta/entity-ui: 598 tests pass.tsc --noEmit, eslint and prettier clean.The failure path itself is not covered by a test. Reproducing a chunk-load failure in jsdom means mocking the dynamic import, which tests the mock more than the boundary. The value here is that a silent hang becomes a visible message.
What to QA
AgentTemplateControlchunk request in the Network tab and reload. Instead of an endless skeleton you get a red "The agent configuration could not load" box naming the cause.