Fix GT label changing to Manual in Ground Truth Job #8938
Conversation
WalkthroughThe pull request addresses an issue with Ground Truth (GT) job label handling during annotation editing. Specifically, the changes focus on preventing the unexpected transformation of the GT label to manual when modifying annotations. The modification occurs in the Changes
Assessment against linked issues
Poem
Finishing Touches
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
🧹 Nitpick comments (2)
cvat-core/src/annotations-objects.ts (2)
57-59: Add explanatory comment for GT source preservation.Consider adding a comment to explain why GT source needs to be preserved, making the intention clear for future maintenance.
if ([Source.AUTO, Source.SEMI_AUTO].includes(currentSource)) { return Source.SEMI_AUTO; } + // Preserve Ground Truth source to prevent GT labels from changing to Manual if (currentSource === Source.GT) { return Source.GT; }
Line range hint
4-4: Add unit tests for computeNewSource function.The function lacks unit tests to verify its behavior with different source types, especially the GT source preservation logic.
Consider adding tests like:
describe('computeNewSource', () => { it('should preserve GT source', () => { expect(computeNewSource(Source.GT)).toBe(Source.GT); }); it('should convert AUTO to SEMI_AUTO', () => { expect(computeNewSource(Source.AUTO)).toBe(Source.SEMI_AUTO); }); it('should preserve SEMI_AUTO source', () => { expect(computeNewSource(Source.SEMI_AUTO)).toBe(Source.SEMI_AUTO); }); it('should default to MANUAL for other sources', () => { expect(computeNewSource(Source.MANUAL)).toBe(Source.MANUAL); }); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
changelog.d/20250113_214825_rahulharpal91_GT_Job_Error.md(1 hunks)cvat-core/src/annotations-objects.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- changelog.d/20250113_214825_rahulharpal91_GT_Job_Error.md
🔇 Additional comments (2)
cvat-core/src/annotations-objects.ts (2)
57-59: LGTM! The fix correctly preserves GT source.The implementation properly maintains the Ground Truth source during annotation modifications, which should fix the issue where GT labels were incorrectly changing to Manual.
57-59: Verify the fix in different editing scenarios.While the implementation looks correct, please verify that the GT source is preserved in various editing scenarios:
- Modifying points/boundaries
- Rotating shapes
- Changing attributes
- Moving annotations
✅ Verification successful
GT source preservation verified across all editing scenarios
The implementation correctly preserves GT source across all shape modifications including points/boundaries, rotation, attributes, and movement operations. The source field is properly protected and tracked in the history for undo/redo operations.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Find all places where computeNewSource is used to ensure consistent behavior # Search for direct function calls echo "Direct function calls:" rg "computeNewSource" -A 3 # Search for source modifications in shape operations echo "\nShape operations affecting source:" ast-grep --pattern 'this.source = $_'Length of output: 9531
|
@zhiltsov-max I have added the patch for the issue #8874. Should I also add tests for this fix? Or it is not necessary ? |
zhiltsov-max
requested review from
klakhov
and removed request for
bsekachev and
nmanovic
Motivation and context
Fixes #8874
How has this been tested?
Here is a demo video showing that the GT label doesn't change to Manual in a Ground Truth Job:
In this video, I have also shown that the Ground Truth label is assigned only when we are annotating in the ground truth job. When we are annotating in a normal job, the annotation has a Manual label or any other depending on the source.
2025-01-13.19-21-19.mp4
Checklist
developbranch(cvat-canvas,
cvat-core,
cvat-data and
cvat-ui)
License
Feel free to contact the maintainers if that's a concern.
Summary by CodeRabbit