Fix appending task bbox to user bbox in annotation nml upload #8280
Conversation
📝 Walkthrough📝 WalkthroughWalkthroughThe changes in the Changes
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🔇 Additional comments (1)CHANGELOG.unreleased.md (1)
The entry accurately describes the fix, follows the changelog format, and provides proper traceability by linking to PR #8280. 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 (1)
app/models/annotation/nml/NmlParser.scala (1)
160-160: Consider using immutable operations instead of mutable state.While changing to
varworks, consider using immutable operations for better maintainability and thread safety:- var userBoundingBoxes = parseBoundingBoxes(parameters \ "userBoundingBox") + val initialBoundingBoxes = parseBoundingBoxes(parameters \ "userBoundingBox")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/models/annotation/nml/NmlParser.scala(1 hunks)
🔇 Additional comments (1)
app/models/annotation/nml/NmlParser.scala (1)
161-163: LGTM: Correct fix for task bbox handling.
The conditional logic correctly ensures that task bounding box is only appended to user bounding boxes when it's not a task upload, fixing the issue mentioned in the PR objectives.
Let's verify the task bbox handling in other parts of the codebase:
✅ Verification successful
Task bounding box handling is consistent across the codebase
The codebase shows consistent handling of task bounding boxes:
- In
NmlParser.scala, task bounding box is only added to user bounding boxes when!isTaskUpload - In
SkeletonTracingService.scala, task bounding box is handled similarly with afromTaskflag that controls the behavior - The task bounding box is properly maintained as a separate field in the data structures (
NmlResults.scala,NmlWriter.scala)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other places where task bounding box is handled
rg -A 3 "taskBoundingBox" --type scala
Length of output: 4526
| else None | ||
| userBoundingBoxes = parseBoundingBoxes(parameters \ "userBoundingBox") | ||
| } yield { | ||
| var userBoundingBoxes = parseBoundingBoxes(parameters \ "userBoundingBox") | ||
| if (!sharedParsingParameters.isTaskUpload) { | ||
| parseTaskBoundingBoxAsUserBoundingBox(parameters \ "taskBoundingBox", userBoundingBoxes) | ||
| .map(asUserBoundingBox => userBoundingBoxes :+ asUserBoundingBox) | ||
| .foreach(asUserBoundingBox => userBoundingBoxes = userBoundingBoxes :+ asUserBoundingBox) |
There was a problem hiding this comment.
The changes to userBoundingBoxes weren't saved. These changes now work close to what the logic was before the #8075 pr. cf:
webknossos/app/models/annotation/nml/NmlParser.scala
Lines 79 to 83 in 5d3d66d
URL of deployed dev instance (used for testing):
Steps to test:
Issues:
(Please delete unneeded items, merge only when none are left open)