Fix: Gt annotations can be shown in standard mode #8675
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 WalkthroughThis pull request introduces a fix for displaying ground truth (GT) annotations in standard mode, addressing an issue where these annotations were incorrectly shown. Additionally, it modifies the settings reducer to handle the closing of jobs, ensuring that the visibility of ground truth annotations is reset when a job is closed. The changes enhance the functionality related to annotation visibility and maintain the overall structure of the reducer. Changes
Assessment against linked issues
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
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: 0
🧹 Outside diff range and nitpick comments (1)
cvat-ui/src/reducers/settings-reducer.ts (1)
Line range hint
462-470: Consider adding documentation for GT visibility behavior.The relationship between job/workspace changes and ground truth visibility might not be immediately obvious to future maintainers. Consider adding a comment explaining why
showGroundTruthneeds to be reset in these cases.Example comment:
case AnnotationActionTypes.CLOSE_JOB: case AnnotationActionTypes.CHANGE_WORKSPACE: { + // Reset ground truth visibility to prevent unintended GT annotation display + // when switching between jobs or workspaces return { ...state, shapes: { ...state.shapes, showGroundTruth: false, }, }; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
changelog.d/20241111_142755_klakhov_fix_show_gt.md(1 hunks)cvat-ui/src/reducers/settings-reducer.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- changelog.d/20241111_142755_klakhov_fix_show_gt.md
🔇 Additional comments (1)
cvat-ui/src/reducers/settings-reducer.ts (1)
Line range hint 462-470: LGTM! Implementation aligns with PR objectives.
The changes correctly reset ground truth annotation visibility when closing a job or changing workspace, which addresses the reported issue of GT annotations being unexpectedly visible in standard mode.
Let's verify all places where ground truth visibility is managed:
✅ Verification successful
Based on the search results, I can now verify the review comment comprehensively. Let me generate the final response:
Ground Truth visibility management is properly implemented across the codebase
The implementation correctly handles Ground Truth visibility state through:
- Proper state initialization (
showGroundTruth: false) - Consistent state updates via
CHANGE_SHOW_GROUND_TRUTHaction - Correct reset behavior on job close and workspace change
- Complete UI integration in components:
- Objects sidebar (
objects-list.tsx,objects-list-header.tsx) - Canvas wrapper (
canvas-wrapper.tsx) - Issues components (
issues-list.tsx,issues-aggregator.tsx)
- Objects sidebar (
The changes align with the existing state management patterns and ensure Ground Truth annotations are properly hidden when expected.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Find all code that manages ground truth visibility to ensure completeness of the fix
# Test 1: Search for direct state management of showGroundTruth
echo "=== Searching for showGroundTruth state management ==="
rg "showGroundTruth"
# Test 2: Search for related action types to ensure proper state transitions
echo "=== Searching for related action types ==="
rg "CHANGE_SHOW_GROUND_TRUTH|CLOSE_JOB|CHANGE_WORKSPACE"
# Test 3: Search for ground truth related UI components
echo "=== Searching for GT-related UI components ==="
rg -t tsx -t ts "groundTruth|GT"
Length of output: 6403
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #8675 +/- ##
===========================================
- Coverage 74.19% 74.19% -0.01%
===========================================
Files 401 401
Lines 43504 43496 -8
Branches 3950 3950
===========================================
- Hits 32278 32272 -6
+ Misses 11226 11224 -2
|
| @@ -460,6 +459,7 @@ export default (state = defaultState, action: AnyAction): SettingsState => { | |||
| }, | |||
| }; | |||
| } | |||
| case AnnotationActionTypes.CLOSE_JOB: | |||
There was a problem hiding this comment.
I would propose to use GET_JOB_SUCCESS instead
|
…8742) <!-- Raise an issue to propose your change (https://github.com/cvat-ai/cvat/issues). It helps to avoid duplication of efforts from multiple independent contributors. Discuss your ideas with maintainers to be sure that changes will be approved and merged. Read the [Contribution guide](https://docs.cvat.ai/docs/contributing/). --> <!-- Provide a general summary of your changes in the Title above --> ### Motivation and context <!-- Why is this change required? What problem does it solve? If it fixes an open issue, please link to the issue here. Describe your changes in detail, add screenshots. --> Test for #8675 ### How has this been tested? <!-- Please describe in detail how you tested your changes. Include details of your testing environment, and the tests you ran to see how your change affects other areas of the code, etc. --> ### Checklist <!-- Go over all the following points, and put an `x` in all the boxes that apply. If an item isn't applicable for some reason, then ~~explicitly strikethrough~~ the whole line. If you don't do that, GitHub will show incorrect progress for the pull request. If you're unsure about any of these, don't hesitate to ask. We're here to help! --> - [x] I submit my changes into the `develop` branch - ~~[ ] I have created a changelog fragment <!-- see top comment in CHANGELOG.md -->~~ - ~~[ ] I have updated the documentation accordingly~~ - [x] I have added tests to cover my changes - ~~[ ] I have linked related issues (see [GitHub docs]( https://help.github.com/en/github/managing-your-work-on-github/linking-a-pull-request-to-an-issue#linking-a-pull-request-to-an-issue-using-a-keyword))~~ - ~~[ ] I have increased versions of npm packages if it is necessary ([cvat-canvas](https://github.com/cvat-ai/cvat/tree/develop/cvat-canvas#versioning), [cvat-core](https://github.com/cvat-ai/cvat/tree/develop/cvat-core#versioning), [cvat-data](https://github.com/cvat-ai/cvat/tree/develop/cvat-data#versioning) and [cvat-ui](https://github.com/cvat-ai/cvat/tree/develop/cvat-ui#versioning))~~ ### License - [x] I submit _my code changes_ under the same [MIT License]( https://github.com/cvat-ai/cvat/blob/develop/LICENSE) that covers the project. Feel free to contact the maintainers if that's a concern. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Bug Fixes** - Improved test reliability by changing the test setup to run before each test case. - **New Features** - Added a test case to ensure ground truth annotations are not visible in the standard annotation view after creation. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Motivation and context
Resolves #8627
To reproduce the issue:
How has this been tested?
Checklist
developbranch[ ] I have updated the documentation accordingly[ ] I have added tests to cover my changes(cvat-canvas,
cvat-core,
cvat-data and
cvat-ui)
License
Feel free to contact the maintainers if that's a concern.
Summary by CodeRabbit
Bug Fixes
Improvements