feat(studio): DD AI config - #961
Conversation
|
This change is part of the following stack: Change managed by git-spice. |
552f2e7 to
bb7a44a
Compare
ed71c61 to
482d7c6
Compare
482d7c6 to
966ae02
Compare
|
📝 WalkthroughWalkthroughAdds an AI-assisted Data Designer flow that generates and validates job configurations, supports fixing validation issues, gates continuation on valid output, and seeds the build route through navigation state. ChangesAI generation and continuation
Generated job seeding
Route integration
Sequence Diagram(s)sequenceDiagram
participant User
participant CreateFilesetStart
participant NewDataDesignerJobRoute
participant DataDesignerJobBuildRoute
User->>CreateFilesetStart: select AI and generate config
CreateFilesetStart->>NewDataDesignerJobRoute: continue with jobRequest
NewDataDesignerJobRoute->>DataDesignerJobBuildRoute: navigate with generatedJobRequest
DataDesignerJobBuildRoute->>DataDesignerJobBuildRoute: validate and seed request
DataDesignerJobBuildRoute-->>User: render initialized builder
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
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: 2
🧹 Nitpick comments (1)
web/packages/studio/src/components/CreateFilesetStart/fixRequest.ts (1)
7-18: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMake repair inputs immutable.
buildFixMessagesonly reads this contract. Mark its fields and arraysreadonly.Proposed fix
export interface FixRequestInput { - prompt: string; - config: string; - errors: string[]; - warnings: string[]; + readonly prompt: string; + readonly config: string; + readonly errors: readonly string[]; + readonly warnings: readonly string[]; } -const bulletList = (items: string[]): string => items.map((item) => `- ${item}`).join('\n'); +const bulletList = (items: readonly string[]): string => + items.map((item) => `- ${item}`).join('\n');As per coding guidelines: “Use
readonlyfor immutable properties in TypeScript interfaces and types.”🤖 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/components/CreateFilesetStart/fixRequest.ts` around lines 7 - 18, Update the FixRequestInput interface fields to readonly, including the errors and warnings array properties, so buildFixMessages receives an immutable repair request contract while preserving the existing field types and behavior.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/CreateFilesetStart/useDescribeWithAi.test.tsx`:
- Around line 12-16: Hoist the mock function used by the useChatCompletion mock
by defining mutateAsync through vi.hoisted before the vi.mock declaration.
Update the existing mutateAsync reference in the test so the mocked hook
continues returning it without accessing an uninitialized const during module
import.
In `@web/packages/studio/src/routes/DataDesignerJobBuildRoute/aiSeed.ts`:
- Around line 67-72: Update seedFromJobRequest to handle an undefined
jobRequest.spec consistently: guard access to spec.config and
spec.config.model_configs, and provide the established empty/default behavior
expected by buildColumnsFromConfig and buildModelsFromConfig. Preserve current
num_records and name fallbacks while ensuring the exported function does not
throw when spec is absent.
---
Nitpick comments:
In `@web/packages/studio/src/components/CreateFilesetStart/fixRequest.ts`:
- Around line 7-18: Update the FixRequestInput interface fields to readonly,
including the errors and warnings array properties, so buildFixMessages receives
an immutable repair request contract while preserving the existing field types
and behavior.
🪄 Autofix (Beta)
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: c13649f0-c1ff-4d78-9a47-ca7397fb8e39
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (19)
web/packages/studio/src/components/CreateFilesetStart/DescribeWithAiPanel.tsxweb/packages/studio/src/components/CreateFilesetStart/GeneratedConfigPanel.tsxweb/packages/studio/src/components/CreateFilesetStart/GeneratedConfigResult.test.tsxweb/packages/studio/src/components/CreateFilesetStart/GeneratedConfigResult.tsxweb/packages/studio/src/components/CreateFilesetStart/StartOptionDetail.tsxweb/packages/studio/src/components/CreateFilesetStart/constants.tsweb/packages/studio/src/components/CreateFilesetStart/fixRequest.tsweb/packages/studio/src/components/CreateFilesetStart/index.test.tsxweb/packages/studio/src/components/CreateFilesetStart/index.tsxweb/packages/studio/src/components/CreateFilesetStart/types.tsweb/packages/studio/src/components/CreateFilesetStart/useDescribeWithAi.test.tsxweb/packages/studio/src/components/CreateFilesetStart/useDescribeWithAi.tsweb/packages/studio/src/components/NewDataDesignerJobForm/constants.tsweb/packages/studio/src/components/NewDataDesignerJobForm/tools.tsweb/packages/studio/src/routes/DataDesignerJobBuildRoute/aiSeed.test.tsweb/packages/studio/src/routes/DataDesignerJobBuildRoute/aiSeed.tsweb/packages/studio/src/routes/DataDesignerJobBuildRoute/index.test.tsxweb/packages/studio/src/routes/DataDesignerJobBuildRoute/index.tsxweb/packages/studio/src/routes/NewDataDesignerJobRoute/index.tsx
| const mutateAsync = vi.fn(); | ||
|
|
||
| vi.mock('@nemo/common/src/hooks/useChatCompletion', () => ({ | ||
| useChatCompletion: () => ({ mutateAsync, isPending: false }), | ||
| })); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
Vitest official documentation: can a vi.mock factory reference a top-level const declared outside the factory, and how should vi.hoisted be used?
💡 Result:
No, a vi.mock factory cannot directly reference top-level const variables declared outside the factory [1][2]. This is because vi.mock calls are hoisted to the top of the file and executed before other code, meaning any external variables referenced within the factory would be in the Temporal Dead Zone (TDZ) or undefined at the time of execution [3][1]. To use variables inside a vi.mock factory, you should use the vi.hoisted utility [1][2]. How to use vi.hoisted: 1. Define variables with vi.hoisted: Call vi.hoisted at the top level of your file, before any vi.mock calls [1][2][4]. This function accepts a factory that returns the values you need to share [3][4]. 2. Reference hoisted values: The values returned by vi.hoisted are moved to the top of the file alongside the mock, making them accessible both inside the vi.mock factory and within your test code [3][4]. Example usage: import { vi, expect, test } from 'vitest'; // Define hoisted variables const { myMockedValue } = vi.hoisted( => ({ myMockedValue: vi.fn, })); // Use hoisted variables inside vi.mock vi.mock('./my-module', => ({ someExport: myMockedValue, })); test('my test', => { myMockedValue.mockReturnValue(123); //... test implementation }); Alternatively, if you do not want to use hoisting, you can use vi.doMock, which is not hoisted and allows access to local variables, though it only affects subsequent dynamic imports [1][2].
Citations:
- 1: https://github.com/vitest-dev/vitest/blob/206e8cff/docs/api/vi.md
- 2: https://github.com/vitest-dev/vitest/blob/v4.1.10/docs/api/vi.md
- 3: https://mergify.com/blog/vitest-vimock-hoisting-traps
- 4: https://vitest.dev/api/vi
Hoist the mock value. vi.mock is hoisted, so this const is read before initialization and can break the test file on import. Use vi.hoisted here.
🤖 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/components/CreateFilesetStart/useDescribeWithAi.test.tsx`
around lines 12 - 16, Hoist the mock function used by the useChatCompletion mock
by defining mutateAsync through vi.hoisted before the vi.mock declaration.
Update the existing mutateAsync reference in the test so the mocked hook
continues returning it without accessing an uninitialized const during module
import.
| export const seedFromJobRequest = (jobRequest: DataDesignerJobRequest): JobBuilderSeed => ({ | ||
| name: jobRequest.name?.trim() || DEFAULT_GENERATED_NAME, | ||
| rows: String(jobRequest.spec?.num_records ?? DEFAULT_GENERATED_ROWS), | ||
| columns: buildColumnsFromConfig(jobRequest.spec.config), | ||
| models: buildModelsFromConfig(jobRequest.spec.config.model_configs), | ||
| }); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Inconsistent null-safety on jobRequest.spec.
jobRequest.spec?.num_records is optional-chained but jobRequest.spec.config right below isn't. If spec is ever undefined (it's unknown-derived data from getGeneratedJobRequestFromState, which only checks 'spec' in request, not that the value is truthy), this exported function throws instead of degrading gracefully. Current callers (index.tsx) guard with generatedRequest?.spec ?, but that's an external contract this function shouldn't rely on.
🛡️ Proposed fix
export const seedFromJobRequest = (jobRequest: DataDesignerJobRequest): JobBuilderSeed => ({
name: jobRequest.name?.trim() || DEFAULT_GENERATED_NAME,
rows: String(jobRequest.spec?.num_records ?? DEFAULT_GENERATED_ROWS),
- columns: buildColumnsFromConfig(jobRequest.spec.config),
- models: buildModelsFromConfig(jobRequest.spec.config.model_configs),
+ columns: buildColumnsFromConfig(jobRequest.spec?.config),
+ models: buildModelsFromConfig(jobRequest.spec?.config?.model_configs),
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export const seedFromJobRequest = (jobRequest: DataDesignerJobRequest): JobBuilderSeed => ({ | |
| name: jobRequest.name?.trim() || DEFAULT_GENERATED_NAME, | |
| rows: String(jobRequest.spec?.num_records ?? DEFAULT_GENERATED_ROWS), | |
| columns: buildColumnsFromConfig(jobRequest.spec.config), | |
| models: buildModelsFromConfig(jobRequest.spec.config.model_configs), | |
| }); | |
| export const seedFromJobRequest = (jobRequest: DataDesignerJobRequest): JobBuilderSeed => ({ | |
| name: jobRequest.name?.trim() || DEFAULT_GENERATED_NAME, | |
| rows: String(jobRequest.spec?.num_records ?? DEFAULT_GENERATED_ROWS), | |
| columns: buildColumnsFromConfig(jobRequest.spec?.config), | |
| models: buildModelsFromConfig(jobRequest.spec?.config?.model_configs), | |
| }); |
🤖 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/DataDesignerJobBuildRoute/aiSeed.ts` around
lines 67 - 72, Update seedFromJobRequest to handle an undefined jobRequest.spec
consistently: guard access to spec.config and spec.config.model_configs, and
provide the established empty/default behavior expected by
buildColumnsFromConfig and buildModelsFromConfig. Preserve current num_records
and name fallbacks while ensuring the exported function does not throw when spec
is absent.
Signed-off-by: Sean Teramae <steramae@nvidia.com>
966ae02 to
9cbed7c
Compare
Signed-off-by: Sean Teramae <steramae@nvidia.com>
|
Closing since other PRs got merged |
1 participant
This PR adds the ability to create a DD config and subsequent DD job with natural language
Signed-off-by: Sean Teramae steramae@nvidia.com
Summary by CodeRabbit
New Features
Bug Fixes