feat(ui): add spec tab to status details#1553
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 WalkthroughWalkthroughThis PR adds a new API endpoint to retrieve YAML DAG specifications for specific DAG runs and introduces corresponding frontend components to display and edit specifications. Changes span backend API generation, endpoint implementation, schema definitions, and UI integration. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
ui/src/features/dags/components/dag-editor/DAGSpecReadOnly.tsx (1)
29-72: Consider: Add props to support editing mode in the future.The component is well-structured and focused. However, if you anticipate needing edit capabilities for certain scenarios (e.g., viewing historical run specs vs editing current DAG specs), consider accepting an optional
readOnlyprop instead of hardcoding it totrueat line 68.🔎 Optional enhancement
type DAGSpecReadOnlyProps = { /** DAG name to fetch the spec for */ dagName: string; /** DAG run ID */ dagRunId: string; /** Additional class name for the container */ className?: string; + /** Whether the editor should be read-only (defaults to true) */ + readOnly?: boolean; }; - function DAGSpecReadOnly({ dagName, dagRunId, className }: DAGSpecReadOnlyProps) { + function DAGSpecReadOnly({ dagName, dagRunId, className, readOnly = true }: DAGSpecReadOnlyProps) { // ... return ( <DAGEditorWithDocs value={data.spec} - readOnly={true} + readOnly={readOnly} className={className} /> ); }ui/src/features/dags/components/dag-editor/DAGEditorWithDocs.tsx (1)
19-32: Consider exporting the props type.The
DAGEditorWithDocsPropstype is well-documented but not exported. If other components need to pass these props programmatically, consider exporting it.🔎 Proposed change
-type DAGEditorWithDocsProps = { +export type DAGEditorWithDocsProps = {ui/src/features/dags/components/dag-editor/DAGSpec.tsx (1)
7-8: Inconsistent React hook import pattern.The file imports only
useEffectdestructured while usingReact.useState,React.useCallback,React.useContext, andReact.useRefelsewhere. This works but is inconsistent. Consider either destructuring all used hooks or usingReact.*uniformly.🔎 Proposed fix for consistency
-import React, { useEffect } from 'react'; +import React, { useCallback, useContext, useEffect, useRef, useState } from 'react';Then update usages throughout the file (e.g.,
useStateinstead ofReact.useState).
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
api/v2/api.gen.goapi/v2/api.yamlinternal/service/frontend/api/v2/dagruns.goui/src/api/v2/schema.tsui/src/features/dags/components/DAGStatus.tsxui/src/features/dags/components/dag-editor/DAGEditorWithDocs.tsxui/src/features/dags/components/dag-editor/DAGSpec.tsxui/src/features/dags/components/dag-editor/DAGSpecReadOnly.tsxui/src/features/dags/components/dag-editor/index.ts
🧰 Additional context used
📓 Path-based instructions (4)
ui/**/*.{ts,tsx,jsx,js}
📄 CodeRabbit inference engine (ui/CLAUDE.md)
ui/**/*.{ts,tsx,jsx,js}: Use developer-centric UI design with high information density, minimal whitespace, compact components, and no unnecessary decorations
Support both light and dark modes for all UI components using Tailwind CSS class pairs likedark:bg-slate-700
NEVER use full-page loading overlays or LoadingIndicator components that hide content - show stale data while fetching updates instead
Use compact modal design with small headers, minimal padding (p-2 or p-3), tight spacing, and support keyboard navigation (arrows, enter, escape)
Use small heights for form elements: select boxes h-7 or smaller, buttons h-7 or h-8, inputs with compact padding (py-0.5 or py-1)
Minimize row heights in tables and lists while maintaining readability, merge related columns, and always handle long text withwhitespace-normal break-words
Use consistent metadata styling withbg-slate-200 dark:bg-slate-700backgrounds and maintain text hierarchy with primary/secondary/muted text colors
Use flexbox-first layouts withmin-h-0andoverflow-hiddento prevent layout breaks, account for fixed elements when setting heights
Maintain keyboard navigation support in all interactive components with appropriate focus indicators and ARIA labels
Files:
ui/src/features/dags/components/dag-editor/DAGSpecReadOnly.tsxui/src/features/dags/components/dag-editor/DAGEditorWithDocs.tsxui/src/features/dags/components/DAGStatus.tsxui/src/features/dags/components/dag-editor/DAGSpec.tsxui/src/features/dags/components/dag-editor/index.tsui/src/api/v2/schema.ts
ui/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
ui/**/*.{ts,tsx}: The React + TypeScript frontend resides inui/, with production bundles copied tointernal/service/frontend/assetsbymake ui
UI code follows ESLint + Prettier (2-space indent) and Tailwind utilities; name React components in PascalCase (JobList.tsx) and hooks withuse*(useJobs.ts)
Files:
ui/src/features/dags/components/dag-editor/DAGSpecReadOnly.tsxui/src/features/dags/components/dag-editor/DAGEditorWithDocs.tsxui/src/features/dags/components/DAGStatus.tsxui/src/features/dags/components/dag-editor/DAGSpec.tsxui/src/features/dags/components/dag-editor/index.tsui/src/api/v2/schema.ts
**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
**/*.go: Backend entrypoint incmd/orchestrates the scheduler and CLI; runtime, persistence, and service layers sit underinternal/*(for exampleinternal/runtime,internal/persistence)
Keep Go filesgofmt/goimportsclean; use tabs, PascalCase for exported symbols (SchedulerClient), lowerCamelCase for locals, andErr...names for package-level errors
Repository linting relies ongolangci-lint; prefer idiomatic Go patterns, minimal global state, and structured logging helpers ininternal/common
Files:
internal/service/frontend/api/v2/dagruns.goapi/v2/api.gen.go
{api/v1,api/v2}/**/*.{proto,yaml,yml,json}
📄 CodeRabbit inference engine (AGENTS.md)
API definitions live in
api/v1andapi/v2; generated server stubs land ininternal/service, while matching TypeScript clients flow intoui/src/api
Files:
api/v2/api.yaml
🧬 Code graph analysis (4)
internal/service/frontend/api/v2/dagruns.go (2)
api/v2/api.gen.go (5)
GetDAGRunSpecRequestObject(6622-6626)GetDAGRunSpecResponseObject(6628-6630)GetDAGRunSpec404JSONResponse(6644-6644)ErrorCodeNotFound(49-49)GetDAGRunSpec200JSONResponse(6632-6635)api/v1/api.gen.go (1)
ErrorCodeNotFound(58-58)
ui/src/features/dags/components/dag-editor/DAGEditorWithDocs.tsx (5)
ui/src/features/dags/components/dag-editor/DAGEditor.tsx (1)
CursorPosition(36-39)ui/src/hooks/useDebouncedValue.ts (1)
useDebouncedValue(6-20)ui/src/hooks/useYamlCursorPath.ts (1)
useYamlCursorPath(313-339)ui/src/lib/utils.ts (1)
cn(4-6)ui/src/components/ui/button.tsx (1)
Button(57-57)
ui/src/features/dags/components/DAGStatus.tsx (1)
ui/src/components/ui/tabs.tsx (1)
Tab(49-49)
api/v2/api.gen.go (2)
internal/common/config/config.go (1)
RemoteNode(240-249)internal/common/logger/tag/tag.go (2)
URL(190-192)Name(271-273)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Test on ubuntu-latest
- GitHub Check: Build
🔇 Additional comments (16)
ui/src/features/dags/components/dag-editor/index.ts (1)
11-11: LGTM! Clean export additions.The new exports for
DAGEditorWithDocsandDAGSpecReadOnlyfollow the existing pattern and properly expose these components for external use.Also applies to: 16-16
api/v2/api.yaml (1)
1632-1668: LGTM! Well-defined API endpoint.The OpenAPI specification for the new
/dag-runs/{name}/{dagRunId}/specendpoint is clear and follows the established patterns. The description correctly indicates it retrieves the spec for a specific DAG-run.ui/src/features/dags/components/DAGStatus.tsx (1)
9-9: LGTM! Consistent tab integration.The new Spec tab is well-integrated:
- Follows the existing tab pattern with icon and label
- Properly typed in the StatusTab union
- Correctly renders DAGSpecReadOnly with appropriate props
- Uses the FileCode icon consistently with the UI design
Also applies to: 32-32, 40-40, 307-314, 415-418
api/v2/api.gen.go (5)
1418-1422: GetDAGRunSpecParams matches existing RemoteNode param patternsThe new
GetDAGRunSpecParamsstruct uses the sameRemoteNode *RemoteNodequery binding and tags as other DAG/DAG-run param types (GetDAGRunDetailsParams,GetDAGSpecParams, etc.). No issues spotted.
2057-2059: Endpoint wiring for GetDAGRunSpec is consistent end‑to‑end
ServerInterface.GetDAGRunSpecand theUnimplementedstub follow the same signature and behavior as neighboring DAG‑run endpoints.ServerInterfaceWrapper.GetDAGRunSpeccorrectly:
- Binds
name/dagRunIdfrom the path usingruntime.BindStyledParameterWithOptions.- Binds optional
remoteNodefrom the query intoGetDAGRunSpecParams.- Sets
ApiTokenScopes/BasicAuthScopesin context like other DAG‑run handlers.HandlerWithOptionsregistersGET /dag-runs/{name}/{dagRunId}/specwith that wrapper, matching the path shape of other DAG‑run routes.Everything looks mechanically identical to the surrounding handlers; no correctness or routing concerns.
Also applies to: 2306-2310, 3357-3408, 5639-5641
6622-6663: Typed request/response for GetDAGRunSpec align with API design
GetDAGRunSpecRequestObjectmirrorsGetDAGRunDetailsRequestObject(path fields + params), so it will integrate cleanly with strict handling.- The response types (
GetDAGRunSpec200JSONResponse,GetDAGRunSpec404JSONResponse,GetDAGRunSpecdefaultJSONResponse) and theirVisit...methods follow the established pattern (content type, status codes, JSON encoding).- 200 response carrying a single
specstring is coherent with the endpoint’s purpose and comments.No issues; types should compile and behave as expected.
8555-8557: Strict server integration for GetDAGRunSpec is correctly plumbed
StrictServerInterface.GetDAGRunSpecuses the right request/response types and matches other strict DAG‑run methods.strictHandler.GetDAGRunSpeccorrectly:
- Builds
GetDAGRunSpecRequestObjectfromname,dagRunId, andGetDAGRunSpecParams.- Wraps the strict handler with middlewares using the standard pattern.
- Validates the returned value against
GetDAGRunSpecResponseObjectand delegates toVisitGetDAGRunSpecResponse.Strict and non‑strict layers appear in sync; no additional changes needed here.
Also applies to: 9248-9274
10555-10790: Updated swaggerSpec blob appears consistent with new endpointThe base64‑encoded, gzipped
swaggerSpecpayload has changed to incorporate the new/dag-runs/{name}/{dagRunId}/specoperation. Since this section is generated byoapi-codegenand not intended for manual edits, the only thing to verify is that it was regenerated from the updated OpenAPI document, not hand‑tweaked. As long as that’s true, this block is fine.Please confirm this file (including
swaggerSpec) was produced by re‑running your usualoapi-codegen/go generatepipeline from the updated schema, so future regenerations remain consistent.ui/src/features/dags/components/dag-editor/DAGEditorWithDocs.tsx (5)
1-14: LGTM - Clean imports and module documentation.The imports are well-organized and the module docstring provides clear context about the component's purpose.
47-55: LGTM - Robust localStorage handling.The sidebar state initialization properly handles environments where localStorage may be unavailable, with appropriate fallback behavior.
57-75: LGTM - Well-optimized cursor tracking.The 150ms debounce prevents excessive re-renders during cursor movement, and the
useCallbackproperly memoizes the handler.
90-103: Keyboard shortcut implementation is correct.The
Ctrl+Shift+Dshortcut is well-implemented with proper cleanup. Note that this shortcut may conflict with browser developer tools bookmarks in some browsers (e.g., Chrome usesCtrl+Shift+Dfor bookmarking), though thepreventDefault()should handle most cases.Consider testing this shortcut across different browsers to ensure it doesn't conflict with browser-specific functionality.
105-152: LGTM - Clean layout following guidelines.The component properly uses flexbox with
min-h-0for layout stability, supports dark mode via semantic color classes, and correctly handles conditional rendering. The empty<div />placeholder (line 126) maintains consistent header layout when onlyheaderActionsis provided.ui/src/api/v2/schema.ts (2)
599-618: Auto-generated path entry looks correct.The new
/dag-runs/{name}/{dagRunId}/specendpoint follows the established URL pattern for DAG-run-specific resources, consistent with sibling endpoints like/dag-runs/{name}/{dagRunId}/log.
3841-3889: Auto-generated operation definition is well-structured.The
getDAGRunSpecoperation correctly mirrors the parameter structure ofgetDAGRunDetailsand provides appropriate response types including a 404 for missing resources.ui/src/features/dags/components/dag-editor/DAGSpec.tsx (1)
391-418: Clean integration with DAGEditorWithDocs.The refactored editor integration:
- Correctly handles editable vs read-only states
- Preserves the existing Save button behavior with keyboard shortcut hint
- Uses appropriate conditional rendering for
headerActionsThe
onChangehandler properly convertsundefinedto empty string, aligning with theonChange?: (value?: string) => voidsignature from DAGEditorWithDocs.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1553 +/- ##
==========================================
+ Coverage 63.41% 63.43% +0.02%
==========================================
Files 215 215
Lines 24166 24229 +63
==========================================
+ Hits 15325 15370 +45
- Misses 7380 7394 +14
- Partials 1461 1465 +4 see 4 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
Summary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.