feat(metadata-instance-editor): remove ai agent selector split and fi… #4226
Conversation
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
WalkthroughRemoved the canUseAIFolderExtractionAgentSelector prop across Metadata Instance Editor components and tests; simplified CascadePolicy AI selector rendering to depend on isAIFolderExtractionEnabled; added selectorAlignment="left" to BoxAiAgentSelectorWithApiContainer; updated stories accordingly; bumped @box/box-ai-agent-selector to ^0.53.0. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant MetadataInstanceEditor
participant Instances
participant Instance
participant CascadePolicy
participant AIAgentSelector
User->>MetadataInstanceEditor: Open editor
MetadataInstanceEditor->>Instances: Render (canUseAIFolderExtraction)
Instances->>Instance: Render
Instance->>CascadePolicy: Render with cascade config
CascadePolicy-->>CascadePolicy: Check shouldShowCascadeOptions && isAIFolderExtractionEnabled
CascadePolicy->>AIAgentSelector: Render (selectorAlignment="left")
User->>AIAgentSelector: Select agent
AIAgentSelector-->>CascadePolicy: onSelectAgent callback
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these settings in your CodeRabbit configuration.
⛔ Files ignored due to path filters (3)
src/features/metadata-instance-editor/__tests__/__snapshots__/Instance.test.js.snapis excluded by!**/*.snapsrc/features/metadata-instance-editor/__tests__/__snapshots__/Instances.test.js.snapis excluded by!**/*.snapsrc/features/metadata-instance-editor/__tests__/__snapshots__/MetadataInstanceEditor.test.js.snapis excluded by!**/*.snap
📒 Files selected for processing (10)
src/features/metadata-instance-editor/CascadePolicy.js(1 hunks)src/features/metadata-instance-editor/CascadePolicy.scss(1 hunks)src/features/metadata-instance-editor/Instance.js(0 hunks)src/features/metadata-instance-editor/Instances.js(0 hunks)src/features/metadata-instance-editor/MetadataInstanceEditor.js(0 hunks)src/features/metadata-instance-editor/__tests__/CascadePolicy.test.js(0 hunks)src/features/metadata-instance-editor/__tests__/Instance.test.js(0 hunks)src/features/metadata-instance-editor/__tests__/Instances.test.js(0 hunks)src/features/metadata-instance-editor/__tests__/MetadataInstanceEditor.test.js(0 hunks)src/features/metadata-instance-editor/stories/tests/CascadePolicy-visual.stories.js(0 hunks)
💤 Files with no reviewable changes (8)
- src/features/metadata-instance-editor/Instance.js
- src/features/metadata-instance-editor/stories/tests/CascadePolicy-visual.stories.js
- src/features/metadata-instance-editor/tests/MetadataInstanceEditor.test.js
- src/features/metadata-instance-editor/Instances.js
- src/features/metadata-instance-editor/MetadataInstanceEditor.js
- src/features/metadata-instance-editor/tests/Instances.test.js
- src/features/metadata-instance-editor/tests/Instance.test.js
- src/features/metadata-instance-editor/tests/CascadePolicy.test.js
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: ahorowitz123
PR: box/box-ui-elements#4102
File: src/features/metadata-instance-editor/Instance.js:647-649
Timestamp: 2025-05-14T17:46:25.370Z
Learning: In the metadata-instance-editor component, `isExistingAIExtractionCascadePolicy` specifically checks if the cascade policy fetched from the backend has AI folder extraction enabled, using props rather than state to reflect the original server-side configuration rather than current UI state.
📚 Learning: 2025-05-14T17:46:25.370Z
Learnt from: ahorowitz123
PR: box/box-ui-elements#4102
File: src/features/metadata-instance-editor/Instance.js:647-649
Timestamp: 2025-05-14T17:46:25.370Z
Learning: In the metadata-instance-editor component, `isExistingAIExtractionCascadePolicy` specifically checks if the cascade policy fetched from the backend has AI folder extraction enabled, using props rather than state to reflect the original server-side configuration rather than current UI state.
Applied to files:
src/features/metadata-instance-editor/CascadePolicy.scsssrc/features/metadata-instance-editor/CascadePolicy.js
⏰ 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: lint_test_build
- GitHub Check: Summary
🔇 Additional comments (1)
src/features/metadata-instance-editor/CascadePolicy.js (1)
195-204: Rendering guard change: confirm it honors existing server-side AI settingsThe selector now renders when isAIFolderExtractionEnabled is true. Per prior learnings (about isExistingAIExtractionCascadePolicy reflecting server-side config), please confirm isAIFolderExtractionEnabled is derived from the canonical server value for existing cascade policies. Otherwise, for an existing AI-enabled policy with a locked toggle, the selector might not render, preventing users from seeing which agent is configured.
If there’s any chance isAIFolderExtractionEnabled reflects only current UI state, consider gating on a prop that encodes the server state for existing policies (e.g., isExistingAIExtractionCascadePolicy), or derive isAIFolderExtractionEnabled from it during initialization so the selector still renders (disabled) for existing AI-enabled policies.
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
src/features/metadata-instance-editor/__tests__/MetadataInstanceEditor.test.js (1)
542-544: Align initial toggle assertion with provided props (isEnabled currently true in base props)getMetadataEditorBaseProps() sets cascadePolicy.isEnabled: true. The test expects the "Enable Cascade Policy" switch to be unchecked before clicking, which contradicts that default. Make the initial state explicit here to avoid brittle assumptions.
Apply this diff to bring the initial state in line with the assertion:
props.editors[0].instance.cascadePolicy.cascadePolicyType = 'ai_extract'; props.editors[0].instance.cascadePolicy.id = null; + // Ensure the switch starts unchecked to match the expectations below + props.editors[0].instance.cascadePolicy.isEnabled = false;
🧹 Nitpick comments (2)
src/features/metadata-instance-editor/__tests__/MetadataInstanceEditor.test.js (2)
547-569: Adopt userEvent.setup() for reliability with async interactionsThe recommended pattern is to use a per-test user instance via userEvent.setup(). It eliminates subtle async timing issues and aligns with the latest testing-library guidance.
Apply this diff:
- const editButton = await screen.findByRole('button', { name: 'Edit Metadata' }, { timeout: 3000 }); - await userEvent.click(editButton); + const user = userEvent.setup(); + const editButton = await screen.findByRole('button', { name: 'Edit Metadata' }, { timeout: 3000 }); + await user.click(editButton); @@ - await userEvent.click(cascadeToggle); + await user.click(cascadeToggle); @@ - await userEvent.click(comboBox); + await user.click(comboBox); @@ - await userEvent.click(enhancedOption); + await user.click(enhancedOption);
557-573: Reduce brittleness of combobox and option role/name queries
- Not all custom Selects expose role="combobox"; some use role="button" + role="listbox"/"option" or menu roles.
- Using the selected value ("Standard"/"Enhanced") as the accessible name of the combobox is implementation-specific and can break with a11y changes.
Consider a more resilient approach:
- Query the combobox without relying on its accessible name, then assert the displayed value.
- Or open the popup and query within the listbox for the option.
Example adjustments:
- const comboBox = screen.getByRole('combobox', { name: 'Standard' }); + // Prefer a broader role query, then assert displayed value separately + const comboBox = screen.getByRole('combobox'); + expect(comboBox).toHaveAccessibleName(/standard/i); @@ - const enhancedOption = await screen.findByRole('option', { name: 'Enhanced' }); + // If the implementation uses listbox/option roles, this will be stable + const enhancedOption = await screen.findByRole('option', { name: /enhanced/i }); @@ - expect(screen.getByRole('combobox', { name: 'Enhanced' })).toBeInTheDocument(); + // After selection, the combobox should reflect the new value + expect(comboBox).toHaveAccessibleName(/enhanced/i);If the underlying control uses a button+menu pattern, swap 'combobox'/'option' for 'button' and 'menuitemradio' or assert by visible text inside the control.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these settings in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
src/features/metadata-instance-editor/__tests__/__snapshots__/Instances.test.js.snapis excluded by!**/*.snap
📒 Files selected for processing (2)
src/features/metadata-instance-editor/__tests__/Instances.test.js(0 hunks)src/features/metadata-instance-editor/__tests__/MetadataInstanceEditor.test.js(1 hunks)
💤 Files with no reviewable changes (1)
- src/features/metadata-instance-editor/tests/Instances.test.js
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: ahorowitz123
PR: box/box-ui-elements#4102
File: src/features/metadata-instance-editor/Instance.js:647-649
Timestamp: 2025-05-14T17:46:25.370Z
Learning: In the metadata-instance-editor component, `isExistingAIExtractionCascadePolicy` specifically checks if the cascade policy fetched from the backend has AI folder extraction enabled, using props rather than state to reflect the original server-side configuration rather than current UI state.
📚 Learning: 2025-05-14T17:46:25.370Z
Learnt from: ahorowitz123
PR: box/box-ui-elements#4102
File: src/features/metadata-instance-editor/Instance.js:647-649
Timestamp: 2025-05-14T17:46:25.370Z
Learning: In the metadata-instance-editor component, `isExistingAIExtractionCascadePolicy` specifically checks if the cascade policy fetched from the backend has AI folder extraction enabled, using props rather than state to reflect the original server-side configuration rather than current UI state.
Applied to files:
src/features/metadata-instance-editor/__tests__/MetadataInstanceEditor.test.js
⏰ 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: lint_test_build
- GitHub Check: Summary
🔇 Additional comments (2)
src/features/metadata-instance-editor/__tests__/MetadataInstanceEditor.test.js (2)
537-574: Good end-to-end coverage of the new agent selector flowThe test exercises the new flow (enable cascade, open the selector, choose "Enhanced") and validates the visible state transition. This is the right place to assert the UI contract post-removal of canUseAIFolderExtractionAgentSelector.
551-555: Verify control role: some Toggle implementations expose role="checkbox", not "switch"If the underlying Toggle uses a checkbox pattern, getByRole('switch', ...) will fail. Consider ensuring the component exposes role="switch", or switch to a resilient query that supports both.
If needed, adjust with a fallback:
- const cascadeToggle = screen.getByRole('switch', { name: 'Enable Cascade Policy' }); + const cascadeToggle = + screen.queryByRole('switch', { name: 'Enable Cascade Policy' }) || + screen.getByRole('checkbox', { name: 'Enable Cascade Policy' });
src/features/metadata-instance-editor/__tests__/MetadataInstanceEditor.test.js
Show resolved
Hide resolved
tjuanitas
reviewed
Aug 15, 2025
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
package.json (1)
298-298: Raising peerDependency to ^0.53.0 is reasonable; call out the peer bump in release notes to aid consumers.
Given 0.x semver, caret allows 0.53.x only, which is appropriate. This may require consumers to upgrade; a brief note in the changelog/migration guide will reduce friction.Would you like me to draft a short release note entry summarizing:
- peerDependencies: @box/box-ai-agent-selector ^0.52.0 → ^0.53.0
- UI change: agent selector left-aligned and legacy gating prop removed?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (4)
src/features/metadata-instance-editor/__tests__/__snapshots__/Instance.test.js.snapis excluded by!**/*.snapsrc/features/metadata-instance-editor/__tests__/__snapshots__/Instances.test.js.snapis excluded by!**/*.snapsrc/features/metadata-instance-editor/__tests__/__snapshots__/MetadataInstanceEditor.test.js.snapis excluded by!**/*.snapyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (10)
package.json(2 hunks)src/features/metadata-instance-editor/CascadePolicy.js(1 hunks)src/features/metadata-instance-editor/Instance.js(0 hunks)src/features/metadata-instance-editor/Instances.js(0 hunks)src/features/metadata-instance-editor/MetadataInstanceEditor.js(0 hunks)src/features/metadata-instance-editor/__tests__/CascadePolicy.test.js(0 hunks)src/features/metadata-instance-editor/__tests__/Instance.test.js(0 hunks)src/features/metadata-instance-editor/__tests__/Instances.test.js(3 hunks)src/features/metadata-instance-editor/__tests__/MetadataInstanceEditor.test.js(1 hunks)src/features/metadata-instance-editor/stories/tests/CascadePolicy-visual.stories.js(0 hunks)
💤 Files with no reviewable changes (6)
- src/features/metadata-instance-editor/stories/tests/CascadePolicy-visual.stories.js
- src/features/metadata-instance-editor/MetadataInstanceEditor.js
- src/features/metadata-instance-editor/tests/Instance.test.js
- src/features/metadata-instance-editor/tests/CascadePolicy.test.js
- src/features/metadata-instance-editor/Instance.js
- src/features/metadata-instance-editor/Instances.js
🚧 Files skipped from review as they are similar to previous changes (3)
- src/features/metadata-instance-editor/tests/Instances.test.js
- src/features/metadata-instance-editor/CascadePolicy.js
- src/features/metadata-instance-editor/tests/MetadataInstanceEditor.test.js
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: ahorowitz123
PR: box/box-ui-elements#4102
File: src/features/metadata-instance-editor/Instance.js:647-649
Timestamp: 2025-05-14T17:46:25.370Z
Learning: In the metadata-instance-editor component, `isExistingAIExtractionCascadePolicy` specifically checks if the cascade policy fetched from the backend has AI folder extraction enabled, using props rather than state to reflect the original server-side configuration rather than current UI state.
📚 Learning: 2025-08-12T18:04:17.698Z
Learnt from: tjuanitas
PR: box/box-ui-elements#4224
File: package.json:296-297
Timestamp: 2025-08-12T18:04:17.698Z
Learning: In the box-ui-elements project, the team is comfortable with raising peerDependency minimum versions when upgrading blueprint-web packages, even if it's a breaking change for consumers.
Applied to files:
package.json
⏰ 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: Summary
- GitHub Check: lint_test_build
🔇 Additional comments (1)
package.json (1)
129-129: Dev/peer dependency bump confirmed; gating prop removal verified
- No references to
canUseAIFolderExtractionAgentSelectorremain.devDependenciesandpeerDependenciesfor@box/box-ai-agent-selectorare both set to^0.53.0.yarn.lockresolves@box/[email protected].- No explicit
selectorAlignment="left"occurrences found—please confirm that the component’s default aligns left or add the prop where it’s rendered.
tjuanitas
approved these changes
Aug 18, 2025
jsenar
approved these changes
Aug 18, 2025
bfoxx1906
pushed a commit
to bfoxx1906/box-ui-elements
that referenced
this pull request
Aug 19, 2025
box#4226) * feat(metadata-instance-editor): remove ai agent selector split and fix styling * fix: tests * fix: tests * feat: use new box ai agent selector with alignment
mergify bot
pushed a commit
that referenced
this pull request
Aug 22, 2025
* feat(timestamped-comments): feature enabled timesetamp support * feat(timestamped-comments): updating stories and adding toggle form * feat(timestamped-comments): fixing flow errors * feat(timestamped-comments): removing unnecessary prop * feat(timestamped-comments): removing formatting * feat(timestamped-comments): removing async modifier * feat(timestamped-comments): removing unnecssary import * feat(timestamped-comments): passing down file prop to BaseComment * feat(metadata-instance-editor): remove ai agent selector split and fi… (#4226) * feat(metadata-instance-editor): remove ai agent selector split and fix styling * fix: tests * fix: tests * feat: use new box ai agent selector with alignment * feat(metadata-view): Implement metadata sidepanel (#4230) * feat(metadata-view): Implement metadata sidepanel * feat(metadata-view): resolve comments * feat(metadata-view): resolve comments * feat(metadata-view): resolve nits * feat(timestamped-comments): addressing PR feedback * feat(timestamped-comments): addressing PR comments * feat(timestamped-comments): addressing PR feedback * feat(timestamped-comments): addressing PR feedback * feat(timestamped-comments): fixing flow error * feat(timestamped-comments): addressing PR feedback * feat(timestamped-comments): fixing unit tests --------- Co-authored-by: jlandisBox <[email protected]> Co-authored-by: Jerry Jiang <[email protected]>
greg-in-a-box
pushed a commit
to greg-in-a-box/box-ui-elements
that referenced
this pull request
Aug 22, 2025
…#4228) * feat(timestamped-comments): feature enabled timesetamp support * feat(timestamped-comments): updating stories and adding toggle form * feat(timestamped-comments): fixing flow errors * feat(timestamped-comments): removing unnecessary prop * feat(timestamped-comments): removing formatting * feat(timestamped-comments): removing async modifier * feat(timestamped-comments): removing unnecssary import * feat(timestamped-comments): passing down file prop to BaseComment * feat(metadata-instance-editor): remove ai agent selector split and fi… (box#4226) * feat(metadata-instance-editor): remove ai agent selector split and fix styling * fix: tests * fix: tests * feat: use new box ai agent selector with alignment * feat(metadata-view): Implement metadata sidepanel (box#4230) * feat(metadata-view): Implement metadata sidepanel * feat(metadata-view): resolve comments * feat(metadata-view): resolve comments * feat(metadata-view): resolve nits * feat(timestamped-comments): addressing PR feedback * feat(timestamped-comments): addressing PR comments * feat(timestamped-comments): addressing PR feedback * feat(timestamped-comments): addressing PR feedback * feat(timestamped-comments): fixing flow error * feat(timestamped-comments): addressing PR feedback * feat(timestamped-comments): fixing unit tests --------- Co-authored-by: jlandisBox <[email protected]> Co-authored-by: Jerry Jiang <[email protected]>
bfoxx1906
pushed a commit
to bfoxx1906/box-ui-elements
that referenced
this pull request
Aug 24, 2025
box#4226) * feat(metadata-instance-editor): remove ai agent selector split and fix styling * fix: tests * fix: tests * feat: use new box ai agent selector with alignment
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
…x styling
AI Agent Selector is now left aligned:
Before:
After:
Summary by CodeRabbit
New Features
Tests
Chores