feat(studio): Stop Preview and show logs tab - #1104
Conversation
|
This change is part of the following stack: Change managed by git-spice. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (15)
🚧 Files skipped from review as they are similar to previous changes (15)
📝 WalkthroughWalkthroughThe changes add model inference defaults and column defaults, support preview cancellation, add phishing fileset templates, and move job logs to a dedicated Logs tab with status-aware initial selection. ChangesData Designer build configuration
Preview cancellation
Data Designer job logs
Phishing fileset templates
Sequence Diagram(s)sequenceDiagram
participant User
participant BuilderToolbar
participant DataDesignerJobBuildRoute
participant usePreview
participant PreviewStream
User->>BuilderToolbar: Click Stop
BuilderToolbar->>DataDesignerJobBuildRoute: Invoke onStopPreview
DataDesignerJobBuildRoute->>usePreview: Invoke stopPreview
usePreview->>PreviewStream: Abort active stream
PreviewStream-->>usePreview: Return AbortError
usePreview-->>BuilderToolbar: Append Preview stopped.
sequenceDiagram
participant DataDesignerJobDetailsRoute
participant JobLogsSection
participant useJobLogs
participant LogViewer
DataDesignerJobDetailsRoute->>JobLogsSection: Render Logs tab
JobLogsSection->>useJobLogs: Load routed job logs
useJobLogs-->>JobLogsSection: Return status and logs
JobLogsSection->>LogViewer: Render logs and download configuration
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
web/packages/studio/src/routes/DataDesignerJobDetailsRoute/index.tsx (1)
160-160: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winGuard the tab value before storing it.
onValueChangesupplies a string. The assertion can store an unsupported value inselectedTab. Narrow it with aJobDetailsTabtype guard.Proposed fix
+const isJobDetailsTab = (value: string): value is JobDetailsTab => + value === 'profile' || value === 'data' || value === 'output' || value === 'logs'; + - onValueChange={(value) => setSelectedTab(value as JobDetailsTab)} + onValueChange={(value) => { + if (isJobDetailsTab(value)) setSelectedTab(value); + }}As per coding guidelines, “Use type guards and narrowing instead of unnecessary type assertions.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/packages/studio/src/routes/DataDesignerJobDetailsRoute/index.tsx` at line 160, Update the onValueChange handler in the selected-tab control to validate the incoming string with a JobDetailsTab type guard before calling setSelectedTab. Only store values recognized as valid JobDetailsTab members, and remove the direct type assertion.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@web/packages/studio/src/components/ModelConfigPanel/index.tsx`:
- Around line 150-162: The max_parallel_requests onChange handler in the
SliderWithTextInput field currently stores decimal values; normalize the
incoming value to an integer before passing it to inferenceParamsField.onChange,
while preserving the existing fallback and field update behavior.
In
`@web/packages/studio/src/routes/DataDesignerJobDetailsRoute/DatasetProfilerSection.tsx`:
- Line 71: Update both descriptions in the profiler empty/loading state to say
“Review the Logs tab for details.” instead of “Review the job logs below for
details.”, preserving the surrounding UI and behavior.
---
Nitpick comments:
In `@web/packages/studio/src/routes/DataDesignerJobDetailsRoute/index.tsx`:
- Line 160: Update the onValueChange handler in the selected-tab control to
validate the incoming string with a JobDetailsTab type guard before calling
setSelectedTab. Only store values recognized as valid JobDetailsTab members, and
remove the direct type assertion.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: ee0daca9-62a0-45b8-9036-25fb43ccc8b2
📒 Files selected for processing (9)
web/packages/studio/src/components/ModelConfigPanel/index.tsxweb/packages/studio/src/components/NewDataDesignerJobForm/usePreview.test.tsxweb/packages/studio/src/components/NewDataDesignerJobForm/usePreview.tsweb/packages/studio/src/constants/constants.tsweb/packages/studio/src/routes/DataDesignerJobBuildRoute/BuilderToolbar.tsxweb/packages/studio/src/routes/DataDesignerJobBuildRoute/index.tsxweb/packages/studio/src/routes/DataDesignerJobDetailsRoute/DatasetProfilerSection.tsxweb/packages/studio/src/routes/DataDesignerJobDetailsRoute/JobLogsSection.tsxweb/packages/studio/src/routes/DataDesignerJobDetailsRoute/index.tsx
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@web/packages/studio/src/components/CreateFilesetStart/templates.ts`:
- Around line 169-189: Update the fileset template around the `analysis` and
`completion` columns to enforce verdict consistency before exporting `analysis`.
Ensure rows are rejected or regenerated unless `is_likely_phishing` exactly
matches whether `label` equals `"phishing"`; preserve the existing `completion`
copy only for valid analyses.
- Around line 19-28: Validate generated corpus rows in the data-designer
persistence flow before DataDesignerResultManager.save_artifacts() accepts them,
rather than relying only on SYNTHETIC_CORPUS_RULES. Reject or regenerate any row
containing clickable/live URLs, domains not ending in .example, real identities,
phone numbers, addresses, or other personal data, and persist only rows that
pass these checks.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 4ccd0757-4805-4827-af2f-9cb5fc13d45c
📒 Files selected for processing (8)
web/packages/studio/src/components/CreateFilesetStart/templates.tsweb/packages/studio/src/components/ModelConfigPanel/index.tsxweb/packages/studio/src/routes/DataDesignerJobBuildRoute/columns.test.tsweb/packages/studio/src/routes/DataDesignerJobBuildRoute/columns.tsweb/packages/studio/src/routes/DataDesignerJobBuildRoute/models.test.tsweb/packages/studio/src/routes/DataDesignerJobBuildRoute/models.tsweb/packages/studio/src/routes/DataDesignerJobBuildRoute/useJobBuilder.tsweb/packages/studio/src/routes/DataDesignerJobDetailsRoute/DatasetProfilerSection.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- web/packages/studio/src/routes/DataDesignerJobDetailsRoute/DatasetProfilerSection.tsx
- web/packages/studio/src/components/ModelConfigPanel/index.tsx
Signed-off-by: Sean Teramae <steramae@nvidia.com>
Signed-off-by: Sean Teramae <steramae@nvidia.com>
ac03e1e to
c67a505
Compare
|
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. |
Signed-off-by: Sean Teramae steramae@nvidia.com
Summary by CodeRabbit
New Features
Bug Fixes