-
-
Notifications
You must be signed in to change notification settings - Fork 10
Add database collection to track semantic domain sense counts per project #4065
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
|
Important Review skippedBot user detected. To trigger a single review, invoke the You can disable this status message by setting the
📝 WalkthroughWalkthroughAdds a semantic-domain count system (model, repo, service), rewires services/controllers to use it, removes legacy frontier-count repo methods, updates DI and frontend API, and adjusts tests and mocks to the new counting approach. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant WordController as "WordController"
participant WordService as "WordService"
participant SDCountService as "SemanticDomainCountService"
participant SDCountRepo as "SemanticDomainCountRepository"
User->>WordController: POST/PUT/DELETE word
WordController->>WordService: create/update/delete request
WordService->>WordService: persist or mutate word(s)
alt domain changes present
WordService->>SDCountService: UpdateCountsForWord/AfterUpdate/ForDeletion
SDCountService->>SDCountService: aggregate per-domain deltas
SDCountService->>SDCountRepo: Increment/Decrement per domain
SDCountRepo-->>SDCountService: updated counts
end
WordService-->>WordController: result / id / status
WordController-->>User: HTTP response
Estimated code review effort🎯 4 (Complex) | ⏱️ ~65 minutes Possibly related PRs
Suggested Labels
Suggested Reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
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. Comment |
|
- Add ProjectSemanticDomainCount model to track sense counts per domain per project - Add ISemanticDomainCountRepository interface and SemanticDomainCountRepository implementation - Add ISemanticDomainCountService interface and SemanticDomainCountService implementation - Integrate count updates into WordService for create, update, delete, and restore operations - Register new services in Startup.cs for dependency injection - Add unit tests for SemanticDomainCountService - Update all existing tests to inject the new SemanticDomainCountService dependency - Add migration method to populate counts from existing Frontier data Co-authored-by: imnasnainaec <6411521+imnasnainaec@users.noreply.github.com>
- Update LiftService to accept ISemanticDomainCountService dependency - Update LiftMerger to accept and use ISemanticDomainCountService - Update SaveImportEntries to call UpdateCountsForWords after creating words - Update test files to provide SemanticDomainCountService to LiftService Co-authored-by: imnasnainaec <6411521+imnasnainaec@users.noreply.github.com>
Done in commits 2ea49c0 and 7220b09:
|
imnasnainaec
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
… logic - Add private GetDomainCounts method to extract domain counts from words - Refactor all methods in SemanticDomainCountService to use GetDomainCounts - Add UpdateCountsForWordDeletion method to simplify deletion operations - Update WordService.Delete to use UpdateCountsForWordDeletion instead of UpdateCountsAfterWordUpdate - Revert WordService.Delete to use Create instead of Add for consistency - Add test for UpdateCountsForWordDeletion method Co-authored-by: imnasnainaec <6411521+imnasnainaec@users.noreply.github.com>
|
commit 8bdcd225ca: commit 2ea49c05dd: |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #4065 +/- ##
===========================================
+ Coverage 74.72% 86.56% +11.83%
===========================================
Files 296 56 -240
Lines 10945 4883 -6062
Branches 1370 597 -773
===========================================
- Hits 8179 4227 -3952
+ Misses 2370 513 -1857
+ Partials 396 143 -253
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@copilot In |
- Create unique compound index on (ProjectId, DomainId) in SemanticDomainCountRepository constructor - Index optimizes queries and enforces uniqueness constraint - Uses Builders.IndexKeys.Ascending for both fields with Unique = true option - Index creation happens at repository initialization Co-authored-by: imnasnainaec <6411521+imnasnainaec@users.noreply.github.com>
Done in commit d1c44e1. Added unique compound index on (ProjectId, DomainId) in the SemanticDomainCountRepository constructor. The index is created using |
…om/sillsdev/TheCombine into copilot/add-sense-count-collection
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as outdated.
This comment was marked as outdated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This pull request introduces a new database collection to track semantic domain sense counts per project, significantly improving performance for statistics queries. Instead of scanning the entire Frontier collection to count domain usage, counts are now maintained incrementally as words are added, updated, or removed.
Changes:
- Added
ProjectSemanticDomainCountmodel andSemanticDomainCountRepositoryfor efficient count storage with compound index on (ProjectId, DomainId) - Implemented
SemanticDomainCountServiceto manage count updates across word lifecycle operations - Refactored
StatisticsServiceto use precomputed counts instead of scanning frontier words - Migrated domain count endpoint from
WordControllertoStatisticsControllerfor better semantic organization - Updated frontend to use new statistics API endpoint
Reviewed changes
Copilot reviewed 36 out of 36 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| Backend/Models/ProjectSemanticDomainCount.cs | New model for storing semantic domain sense counts per project |
| Backend/Repositories/SemanticDomainCountRepository.cs | Repository with atomic operations for count management |
| Backend/Services/SemanticDomainCountService.cs | Service layer managing count updates during word operations |
| Backend/Services/WordService.cs | Integrated count updates for word creation, update, deletion, and restoration |
| Backend/Services/StatisticsService.cs | Refactored to use precomputed counts for better performance |
| Backend/Services/LiftService.cs | Updated to use WordService instead of WordRepository for LIFT imports |
| Backend/Services/MergeService.cs | Updated to use new WordService methods that maintain counts |
| Backend/Controllers/StatisticsController.cs | Added GetDomainCount endpoint |
| Backend/Controllers/WordController.cs | Removed GetDomainWordCount endpoint (moved to Statistics) |
| Backend/Interfaces/* | Added new interfaces and updated existing ones |
| src/api/api/statistics-api.ts | Added getDomainCount API client method |
| src/api/api/word-api.ts | Removed getDomainWordCount API client method |
| src/backend/index.ts | Updated to use statistics API for domain counts |
| src/components/TreeView/TreeDepiction/DomainCountBadge.tsx | Updated to use new getDomainCount function |
| Backend.Tests/* | Comprehensive test coverage for new functionality |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Implementation Complete
ProjectSemanticDomainCountwith projectId, domainId, and count fieldsSemanticDomainCountRepositoryfor database operationsSemanticDomainCountServiceto handle count updatesWordServiceto update counts after words are added/updated/removedSummary by CodeRabbit
Release Notes
New Features
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.
This change is
Summary by CodeRabbit
New Features
Refactor
Tests