Clear CompoundAnnotation Cache on Task Changes #8756
Conversation
📝 WalkthroughWalkthroughThe changes introduce cache invalidation logic for compound annotation views when tasks are finished, cancelled, or reopened. This involves updating controllers and services to clear related cached annotations, renaming a caching flag for clarity, and adding a cache removal method. These updates ensure compound annotation data remains current. Changes
Assessment against linked issues
Poem
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. 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 (
|
fm3
changed the title
![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)
unreleased_changes/8756.md (1)
1-2: Apply style improvements for better readability.The static analysis tools have identified valid style suggestions:
-### Fixed -- Fixed that compound (merged) views of tasks, projects and task types would sometimes show outdated annotation data, leaving out the data from recently finished tasks annotations. +### Fixed +- Fixed an issue where compound (merged) views of tasks, projects and task types would sometimes show outdated annotation data, omitting the data from recently finished task annotations.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
app/controllers/AnnotationController.scala(4 hunks)app/models/annotation/AnnotationService.scala(1 hunks)app/models/annotation/AnnotationStore.scala(3 hunks)app/models/annotation/handler/AnnotationInformationHandler.scala(1 hunks)app/models/annotation/handler/SavedTracingInformationHandler.scala(1 hunks)app/models/task/TaskService.scala(3 hunks)unreleased_changes/8756.md(1 hunks)
🧰 Additional context used
🧠 Learnings (5)
app/models/annotation/handler/SavedTracingInformationHandler.scala (3)
Learnt from: frcroth
PR: scalableminds/webknossos#8236
File: webknossos-datastore/app/com/scalableminds/webknossos/datastore/services/mesh/NeuroglancerPrecomputedMeshFileService.scala:161-166
Timestamp: 2025-04-28T14:18:04.368Z
Learning: In Scala for-comprehensions with the Fox error handling monad, `Fox.fromBool()` expressions should use the `<-` binding operator instead of the `=` assignment operator to properly propagate error conditions. Using `=` will cause validation failures to be silently ignored.
Learnt from: frcroth
PR: scalableminds/webknossos#8236
File: webknossos-datastore/app/com/scalableminds/webknossos/datastore/services/mesh/MeshFileService.scala:170-173
Timestamp: 2025-04-23T08:51:57.756Z
Learning: In the webknossos codebase, classes extending `FoxImplicits` have access to an implicit conversion from `Option[A]` to `Fox[A]`, where `None` is converted to an empty Fox that fails gracefully in for-comprehensions.
Learnt from: MichaelBuessemeyer
PR: scalableminds/webknossos#8352
File: app/models/organization/CreditTransactionService.scala:0-0
Timestamp: 2025-01-27T12:06:42.865Z
Learning: In Scala's for-comprehension with Fox (Future-like type), the `<-` operator ensures sequential execution. If any step fails, the entire chain short-circuits and returns early, preventing subsequent operations from executing. This makes it safe to perform validation checks before database operations.
app/models/annotation/handler/AnnotationInformationHandler.scala (1)
Learnt from: frcroth
PR: scalableminds/webknossos#8236
File: webknossos-datastore/app/com/scalableminds/webknossos/datastore/services/mesh/MeshFileService.scala:170-173
Timestamp: 2025-04-23T08:51:57.756Z
Learning: In the webknossos codebase, classes extending `FoxImplicits` have access to an implicit conversion from `Option[A]` to `Fox[A]`, where `None` is converted to an empty Fox that fails gracefully in for-comprehensions.
app/models/annotation/AnnotationService.scala (1)
Learnt from: MichaelBuessemeyer
PR: scalableminds/webknossos#8352
File: app/models/organization/CreditTransactionService.scala:0-0
Timestamp: 2025-01-27T12:06:42.865Z
Learning: In Scala's for-comprehension with Fox (Future-like type), the `<-` operator ensures sequential execution. If any step fails, the entire chain short-circuits and returns early, preventing subsequent operations from executing. This makes it safe to perform validation checks before database operations.
app/models/task/TaskService.scala (3)
Learnt from: frcroth
PR: scalableminds/webknossos#8236
File: webknossos-datastore/app/com/scalableminds/webknossos/datastore/services/mesh/MeshFileService.scala:170-173
Timestamp: 2025-04-23T08:51:57.756Z
Learning: In the webknossos codebase, classes extending `FoxImplicits` have access to an implicit conversion from `Option[A]` to `Fox[A]`, where `None` is converted to an empty Fox that fails gracefully in for-comprehensions.
Learnt from: MichaelBuessemeyer
PR: scalableminds/webknossos#8352
File: app/models/organization/CreditTransactionService.scala:0-0
Timestamp: 2025-01-27T12:06:42.865Z
Learning: In Scala's for-comprehension with Fox (Future-like type), the `<-` operator ensures sequential execution. If any step fails, the entire chain short-circuits and returns early, preventing subsequent operations from executing. This makes it safe to perform validation checks before database operations.
Learnt from: frcroth
PR: scalableminds/webknossos#8236
File: webknossos-datastore/app/com/scalableminds/webknossos/datastore/services/mesh/NeuroglancerPrecomputedMeshFileService.scala:161-166
Timestamp: 2025-04-28T14:18:04.368Z
Learning: In Scala for-comprehensions with the Fox error handling monad, `Fox.fromBool()` expressions should use the `<-` binding operator instead of the `=` assignment operator to properly propagate error conditions. Using `=` will cause validation failures to be silently ignored.
app/controllers/AnnotationController.scala (1)
Learnt from: frcroth
PR: scalableminds/webknossos#8609
File: app/models/dataset/Dataset.scala:753-775
Timestamp: 2025-05-12T13:07:29.637Z
Learning: In the `updateMags` method of DatasetMagsDAO (Scala), the code handles different dataset types distinctly:
1. Non-WKW datasets have `magsOpt` populated and use the first branch which includes axisOrder, channelIndex, and credentialId.
2. WKW datasets will have `wkwResolutionsOpt` populated and use the second branch which includes cubeLength.
3. The final branch is a fallback for legacy data.
This ensures appropriate fields are populated for each dataset type.
🧬 Code Graph Analysis (2)
app/models/annotation/handler/SavedTracingInformationHandler.scala (1)
app/models/annotation/handler/AnnotationInformationHandler.scala (1)
useCache(35-35)
app/models/annotation/AnnotationStore.scala (2)
app/models/annotation/handler/AnnotationInformationHandler.scala (1)
useCache(35-35)app/models/annotation/AnnotationIdentifier.scala (1)
toUniqueString(11-14)
🪛 LanguageTool
unreleased_changes/8756.md
[duplication] ~1-~1: Possible typo: you repeated a word.
Context: ### Fixed - Fixed that compound (merged) views of tasks, ...
(ENGLISH_WORD_REPEAT_RULE)
[style] ~2-~2: To strengthen your wording, consider replacing the phrasal verb “leave out”.
Context: ...ometimes show outdated annotation data, leaving out the data from recently finished tasks a...
(OMIT_EXCLUDE)
🔇 Additional comments (12)
app/models/annotation/handler/AnnotationInformationHandler.scala (1)
35-35: LGTM! Clear naming improvement.The rename from
cachetouseCachemakes the method's intent more explicit and aligns with similar changes across the codebase.app/models/annotation/handler/SavedTracingInformationHandler.scala (1)
27-27: LGTM! Consistent with trait method rename.The variable rename maintains consistency with the trait method and preserves the existing behavior.
app/models/annotation/AnnotationStore.scala (2)
28-28: LGTM! Consistent naming updates.The updates from
handler.cachetohandler.useCachemaintain consistency with the trait and class renames.Also applies to: 40-40
64-64: LGTM! Clean cache removal implementation.The new
removeFromCachemethod is well-implemented, leveraging the existingtoUniqueStringmethod for consistent key generation. The simple signature makes it easy to use from the cache clearing logic.app/models/annotation/AnnotationService.scala (1)
316-316: LGTM! Proper cache invalidation integration.The cache clearing logic is correctly integrated using
Fox.runOptionalto conditionally clear the compound cache only for task annotations. The placement after state updates ensures consistency, and the async execution avoids blocking the main workflow.app/models/task/TaskService.scala (3)
8-8: Import addition looks correct.The import of
AnnotationStoreandAnnotationIdentifieris properly added to support the new cache clearing functionality.
22-22: Constructor dependency injection is properly implemented.The addition of
annotationStore: AnnotationStoreandtaskDAO: TaskDAOdependencies follows the established pattern in the codebase.Also applies to: 27-27
93-99: Cache clearing method is well-implemented.The
clearCompoundCachemethod correctly:
- Fetches the task using
GlobalAccessContext(appropriate for internal operations)- Removes three related cache entries for compound annotations
- Uses Fox for proper error handling and sequential execution
- Follows established patterns in the codebase
The logic ensures that all compound views (task, project, and task type) are invalidated when a task annotation changes state.
app/controllers/AnnotationController.scala (4)
20-20: Import addition is correct.The import of
TaskServiceis properly added to support the new cache clearing functionality.
55-55: Constructor dependency injection follows established patterns.The addition of
taskService: TaskServicedependency is properly implemented.
162-162: Cache clearing in reopen method is correctly implemented.The use of
Fox.runOptional(annotation._task)(taskService.clearCompoundCache)properly clears the compound cache only when a task ID exists, which is appropriate for maintaining cache consistency after reopening an annotation.
303-313: Cancel method refactoring improves robustness and cache consistency.The changes improve the implementation in several ways:
- Pattern matching on
(annotation._task, annotation.typ)ensures task annotations actually have a task ID- Conditional cache clearing only for previously finished annotations is logical since only finished annotations would affect compound caches
- The sequential execution using Fox ensures proper error handling
The logic correctly handles the cache invalidation before state updates, which is appropriate since the cache will be rebuilt when needed.
When finishing or cancelling a task annotation, clear the compound annotation caches that this annotation influences.
That way, when reloading the compound annotation afterwards, the new data is visible there immediately.
Note that the tracingstore-side caches don’t need to be cleared, as the wk side will decide to put new data there, containing the new merged annotationProto (with all-new tracingIds for the layers).
URL of deployed dev instance (used for testing):
Steps to test:
Issues:
$PR_NUMBER.mdfile inunreleased_changesor use./tools/create-changelog-entry.py)