feat(dashboards): discriminate widget batch-add openapi schema (4/4)#61457
Open
MattPua wants to merge 3 commits into
Open
Conversation
Contributor
|
Size Change: 0 B Total Size: 81.9 MB ℹ️ View Unchanged
|
Contributor
Prompt To Fix All With AIFix the following 1 code review issue. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 1
products/dashboards/frontend/generated/api.zod.ts:564-570
**Duplicate `.enum()` call breaks the Zod schema at runtime**
The generated `widget_type` field calls `.enum(['error_tracking_list']).describe('...').enum(['error_tracking_list']).describe('...')`. In Zod v3, `.enum` on a `ZodEnum` instance is a getter property that returns `{ error_tracking_list: 'error_tracking_list' }` — a plain object, not a function. The second `.enum(['error_tracking_list'])` invocation would throw `TypeError: (...).enum is not a function` when the module is first loaded, making the entire schema unusable.
This pattern appears twice here (lines 564–570 for `error_tracking_list` and lines 689–695 for `session_replay_list`) and identically in `services/mcp/src/generated/dashboards/api.ts` (lines 530–536 and 655–661). The root cause is that drf-spectacular emits two `description` entries for the `ChoiceField` (one for the choice-value representation and one for `help_text`), and the Orval/Zod generator renders each as a separate `.enum(...).describe(...)` chain. The fix should be in the OpenAPI output or in the Orval code-generator config, not in these generated files.
Reviews (1): Last reviewed commit: "feat(dashboards): discriminate widget ba..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+564
to
+570
| widget_type: zod | ||
| .enum(['error_tracking_list']) | ||
| .describe('\* `error_tracking_list` - error_tracking_list') | ||
| .enum(['error_tracking_list']) | ||
| .describe( | ||
| 'Widget type identifier. Supported values: error_tracking_list, session_replay_list. Use dashboard-widget-catalog-list for config_schema_hints per type.\n\n\* `error_tracking_list` - error_tracking_list' | ||
| ), |
Contributor
There was a problem hiding this comment.
.enum() call breaks the Zod schema at runtime
The generated widget_type field calls .enum(['error_tracking_list']).describe('...').enum(['error_tracking_list']).describe('...'). In Zod v3, .enum on a ZodEnum instance is a getter property that returns { error_tracking_list: 'error_tracking_list' } — a plain object, not a function. The second .enum(['error_tracking_list']) invocation would throw TypeError: (...).enum is not a function when the module is first loaded, making the entire schema unusable.
This pattern appears twice here (lines 564–570 for error_tracking_list and lines 689–695 for session_replay_list) and identically in services/mcp/src/generated/dashboards/api.ts (lines 530–536 and 655–661). The root cause is that drf-spectacular emits two description entries for the ChoiceField (one for the choice-value representation and one for help_text), and the Orval/Zod generator renders each as a separate .enum(...).describe(...) chain. The fix should be in the OpenAPI output or in the Orval code-generator config, not in these generated files.
Prompt To Fix With AI
This is a comment left during a code review.
Path: products/dashboards/frontend/generated/api.zod.ts
Line: 564-570
Comment:
**Duplicate `.enum()` call breaks the Zod schema at runtime**
The generated `widget_type` field calls `.enum(['error_tracking_list']).describe('...').enum(['error_tracking_list']).describe('...')`. In Zod v3, `.enum` on a `ZodEnum` instance is a getter property that returns `{ error_tracking_list: 'error_tracking_list' }` — a plain object, not a function. The second `.enum(['error_tracking_list'])` invocation would throw `TypeError: (...).enum is not a function` when the module is first loaded, making the entire schema unusable.
This pattern appears twice here (lines 564–570 for `error_tracking_list` and lines 689–695 for `session_replay_list`) and identically in `services/mcp/src/generated/dashboards/api.ts` (lines 530–536 and 655–661). The root cause is that drf-spectacular emits two `description` entries for the `ChoiceField` (one for the choice-value representation and one for `help_text`), and the Orval/Zod generator renders each as a separate `.enum(...).describe(...)` chain. The fix should be in the OpenAPI output or in the Orval code-generator config, not in these generated files.
How can I resolve this? If you propose a fix, please make it concise.
Contributor
There was a problem hiding this comment.
This seems fair, but not sure why there is a duplicate enum/describe thing here 😕, maybe it's an outdated generated artefact?
MattPua
force-pushed
the
05-28-feat_dashboards_add_mcp_tools_for_dashboard_widgets
branch
from
d117cbb to
30b777e
Compare
MattPua
changed the base branch from
05-28-feat_dashboards_add_mcp_tools_for_dashboard_widgets
to
graphite-base/61457
MattPua
force-pushed
the
feat/dashboards-discriminated-widget-openapi
branch
from
e2de932 to
5f314de
Compare
MattPua
force-pushed
the
graphite-base/61457
branch
from
d117cbb to
2484fe6
Compare
MattPua
changed the base branch from
graphite-base/61457
to
06-03-feat_dashboards_add_widget_delivery_slos
This was referenced Jun 3, 2026
Member
Author
2 tasks
MattPua
changed the title
![stamphog[bot]](https://avatars.githubusercontent.com/in/3000332?s=60&v=4){kind=small}
There was a problem hiding this comment.
Gates denied: this PR modifies public API contracts (OpenAPI schema discrimination for widget batch-add) and spans generated types across dashboards frontend and the MCP service — changes that require human review to validate correctness and downstream compatibility.
MattPua
force-pushed
the
feat/dashboards-discriminated-widget-openapi
branch
from
9b02dc0 to
be2f3ac
Compare
MattPua
force-pushed
the
06-03-feat_dashboards_add_widget_delivery_slos
branch
from
2484fe6 to
285d4bc
Compare
MattPua
force-pushed
the
feat/dashboards-discriminated-widget-openapi
branch
from
be2f3ac to
c64c892
Compare
MattPua
force-pushed
the
06-03-feat_dashboards_add_widget_delivery_slos
branch
from
285d4bc to
72f9878
Compare
MattPua
force-pushed
the
feat/dashboards-discriminated-widget-openapi
branch
from
c64c892 to
936dc70
Compare
MattPua
force-pushed
the
06-03-feat_dashboards_add_widget_delivery_slos
branch
from
72f9878 to
504e583
Compare
MattPua
changed the title
MattPua
force-pushed
the
06-03-feat_dashboards_add_widget_delivery_slos
branch
from
504e583 to
65b7b63
Compare
MattPua
force-pushed
the
feat/dashboards-discriminated-widget-openapi
branch
from
936dc70 to
12a8e9a
Compare
MattPua
force-pushed
the
06-03-feat_dashboards_add_widget_delivery_slos
branch
from
65b7b63 to
a17b4ab
Compare
MattPua
force-pushed
the
feat/dashboards-discriminated-widget-openapi
branch
from
12a8e9a to
3d71e86
Compare
Contributor
|
⏭️ Skipped snapshot commit because branch advanced to The new commit will trigger its own snapshot update workflow. If you expected this workflow to succeed: This can happen due to concurrent commits. To get a fresh workflow run, either:
|
MattPua
force-pushed
the
feat/dashboards-discriminated-widget-openapi
branch
from
3d71e86 to
4c398d5
Compare
MattPua
force-pushed
the
06-03-feat_dashboards_add_widget_delivery_slos
branch
from
a17b4ab to
6f79ef2
Compare
MattPua
force-pushed
the
feat/dashboards-discriminated-widget-openapi
branch
from
4c398d5 to
2d1ad74
Compare
MattPua
force-pushed
the
06-03-feat_dashboards_add_widget_delivery_slos
branch
from
6f79ef2 to
4a0a6c3
Compare
Contributor
|
⏭️ Skipped snapshot commit because branch advanced to The new commit will trigger its own snapshot update workflow. If you expected this workflow to succeed: This can happen due to concurrent commits. To get a fresh workflow run, either:
|
Contributor
Query snapshots: Backend query snapshots updatedChanges: 2 snapshots (2 modified, 0 added, 0 deleted) What this means:
Next steps:
|
vdekrijger
approved these changes
Jun 4, 2026
Contributor
vdekrijger
left a comment
vdekrijger
left a comment
There was a problem hiding this comment.
Same comment as the previous one regarding the .ambr ones, but otherwise LGTM! Just a few notes from the bots that seem fair.
Comment on lines
+564
to
+570
| widget_type: zod | ||
| .enum(['error_tracking_list']) | ||
| .describe('\* `error_tracking_list` - error_tracking_list') | ||
| .enum(['error_tracking_list']) | ||
| .describe( | ||
| 'Widget type identifier. Supported values: error_tracking_list, session_replay_list. Use dashboard-widget-catalog-list for config_schema_hints per type.\n\n\* `error_tracking_list` - error_tracking_list' | ||
| ), |
Contributor
There was a problem hiding this comment.
This seems fair, but not sure why there is a duplicate enum/describe thing here 😕, maybe it's an outdated generated artefact?
| widget_type: zod | ||
| .enum(['session_replay_list']) | ||
| .describe('\* `session_replay_list` - session_replay_list') | ||
| .enum(['session_replay_list']) |
Contributor
There was a problem hiding this comment.
This also has the udplicate enum / describe thing
| widget_type: zod | ||
| .enum(['session_replay_list']) | ||
| .describe('* `session_replay_list` - session_replay_list') | ||
| .enum(['session_replay_list']) |
Contributor
There was a problem hiding this comment.
Just flagging this one as well as the bots did not flag this duplicate (probably due to it being implicitly referenced by their comments already).
MattPua
force-pushed
the
06-03-feat_dashboards_add_widget_delivery_slos
branch
from
5a5a735 to
26e655d
Compare
MattPua
force-pushed
the
feat/dashboards-discriminated-widget-openapi
branch
from
9638929 to
d96d2ae
Compare
Use widget_type-discriminated polymorphic request serializers for dashboard-widgets-batch-add so generated zod/MCP schemas tie config shape to widget_type. Runtime validation unchanged. Co-authored-by: Cursor <cursoragent@cursor.com>
Assert discriminator keys against EXPECTED_WIDGET_TYPES, reuse widget flag patch targets, and patch immutability test with a valid alternate type. Co-authored-by: Cursor <cursoragent@cursor.com>
MattPua
force-pushed
the
06-03-feat_dashboards_add_widget_delivery_slos
branch
from
26e655d to
a210144
Compare
MattPua
force-pushed
the
feat/dashboards-discriminated-widget-openapi
branch
from
d96d2ae to
081b88d
Compare
Contributor
|
⏭️ Skipped snapshot commit because branch advanced to The new commit will trigger its own snapshot update workflow. If you expected this workflow to succeed: This can happen due to concurrent commits. To get a fresh workflow run, either:
|
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
2 participants
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.
Problem
MCP agents calling
dashboard-widgets-batch-addsaw widgetconfigas an untyped JSON blob (or an uncorrelated config union). That made it easy to send replay config with an error-trackingwidget_typewithout schema feedback.Changes
AddDashboardWidgetRequestserializers keyed bywidget_type(batch exports pattern)widgets_batch@extend_schema(request=AddDashboardWidgetsBatchRequestOpenApiSerializer)while keeping runtime validation on the existing serializerwidget_typetoChoiceFieldoverEXPECTED_WIDGET_TYPEShogli build:openapi)How did you test this code?
Agent-authored. Ran:
./bin/hogli test products/dashboards/backend/api/test/test_dashboard_widgets.py::TestDashboardWidgetOpenApiSchema./bin/hogli build:openapiAutomatic notifications
Docs update
🤖 Agent context
Cursor agent stacked this on the MCP widgets PR. Chose a separate OpenAPI-only batch-add serializer so runtime widget validation stays unchanged while spectacular emits a
widget_typediscriminator with per-type config shapes. PATCHtiles[].widgetremains untyped in OpenAPI — follow-up if we wantdashboard-updateagent ergonomics too.Made with Cursor