fix(studio): DD embedding template fix - #661
Conversation
|
This change is part of the following stack:
Change managed by git-spice. |
2439638 to
bfe5fce
Compare
6bafee5 to
8057b7b
Compare
bfe5fce to
452fdd8
Compare
8057b7b to
cc20eba
Compare
452fdd8 to
d0fdb0b
Compare
cc20eba to
0f94702
Compare
Signed-off-by: Sean Teramae <steramae@nvidia.com>
Signed-off-by: Sean Teramae <steramae@nvidia.com>
Signed-off-by: Sean Teramae <steramae@nvidia.com>
0f94702 to
04570fd
Compare
|
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 (2)
📝 WalkthroughWalkthroughThe PR centralizes log auto-scrolling, adds embedding inference support, requires JSON scores for LLM judge columns, updates model fixtures, and standardizes Data Designer icons. ChangesScroll behavior
Data Designer model and column configuration
Data Designer icon updates
Sequence Diagram(s)sequenceDiagram
participant LogViewer
participant useStickToBottom
participant ScrollContainer
participant MutationObserver
LogViewer->>useStickToBottom: configure enabled and resetKey
useStickToBottom->>ScrollContainer: attach ref and scroll listener
useStickToBottom->>MutationObserver: observe content mutations
MutationObserver->>ScrollContainer: scroll to bottom when auto-scroll is active
LogViewer->>useStickToBottom: scrollToBottom()
useStickToBottom->>ScrollContainer: set scrollTop to computed bottom
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: 4
🧹 Nitpick comments (1)
web/packages/common/src/hooks/useStickToBottom/index.ts (1)
4-4: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse type-only imports consistently.
web/packages/common/src/hooks/useStickToBottom/index.ts#L4-L4: importRefObjectusingimport type.web/packages/common/src/hooks/useStickToBottom/index.test.tsx#L7-L7: importFCusingimport type.web/packages/common/src/components/LogViewer/index.tsx#L19-L19: splitFCfrom the runtime React imports.As per coding guidelines, “Use
import typefor type-only imports in TypeScript.”🤖 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/common/src/hooks/useStickToBottom/index.ts` at line 4, Use type-only imports for RefObject in web/packages/common/src/hooks/useStickToBottom/index.ts:4-4 and FC in web/packages/common/src/hooks/useStickToBottom/index.test.tsx:7-7; in web/packages/common/src/components/LogViewer/index.tsx:19-19, split FC from the runtime React imports into an import type declaration.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/common/src/hooks/useStickToBottom/index.test.tsx`:
- Around line 50-56: Update the test harness used by “scrollToBottom() does not
throw before the element is attached” to support omitting the ref-attached div,
then render that unattached configuration before clicking the scroll button.
Keep the assertion that invoking scrollToBottom resolves without throwing, and
preserve existing attached-element test behavior.
In `@web/packages/common/src/hooks/useStickToBottom/index.ts`:
- Around line 11-15: Update the useStickToBottom hook so initialization and
resetKey changes both scroll the container to the bottom and re-arm
shouldAutoScrollRef when stick-to-bottom behavior is enabled. Ensure the
observer recreation path resets the auto-scroll state, and pin existing content
immediately on initial setup so previews open at the bottom.
In `@web/packages/studio/src/components/CreateFilesetStart/templates.ts`:
- Around line 340-343: Update the inferenceParams in the fileset template to set
extra_body.input_type to 'passage' instead of 'query', while preserving the
existing embedding generation and encoding settings.
In `@web/packages/studio/src/routes/DataDesignerJobBuildRoute/columns.ts`:
- Around line 163-170: Update the scores column definition in
DataDesignerJobBuildRoute so its JSON validation requires an array rather than
accepting arbitrary JSON values. Preserve the required field behavior and ensure
objects, null, and primitives are rejected before serialization into the job
configuration.
---
Nitpick comments:
In `@web/packages/common/src/hooks/useStickToBottom/index.ts`:
- Line 4: Use type-only imports for RefObject in
web/packages/common/src/hooks/useStickToBottom/index.ts:4-4 and FC in
web/packages/common/src/hooks/useStickToBottom/index.test.tsx:7-7; in
web/packages/common/src/components/LogViewer/index.tsx:19-19, split FC from the
runtime React imports into an import type declaration.
🪄 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: 5c9fc9f1-caab-417d-b8b1-ce769fda0f48
📒 Files selected for processing (14)
web/packages/common/src/components/LogViewer/index.tsxweb/packages/common/src/hooks/useStickToBottom/index.test.tsxweb/packages/common/src/hooks/useStickToBottom/index.tsweb/packages/studio/src/components/AddModelPalette/AddModelPalette.stories.tsxweb/packages/studio/src/components/CreateFilesetStart/templates.tsweb/packages/studio/src/components/dataViews/DataDesignerJobsDataView/index.tsxweb/packages/studio/src/constants/constants.tsweb/packages/studio/src/mocks/evaluation/configs.tsweb/packages/studio/src/routes/DataDesignerJobBuildRoute/BuilderDetailsPanel.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/WorkspaceLayout/WorkspaceSideNav.tsx
- templates: use input_type 'passage' for the passage-embedding column - columns: require a JSON array for llm-judge 'scores' field - useStickToBottom: pin existing content and re-arm auto-scroll on mount/resetKey - test: exercise the truly-unattached ref path for scrollToBottom Signed-off-by: Sean Teramae <steramae@nvidia.com>
|
This PR fixes the embedding template.
Signed-off-by: Sean Teramae steramae@nvidia.com
Summary by CodeRabbit
scoresconfiguration support for LLM judge columns, with validation that expects a JSON array.