-
Notifications
You must be signed in to change notification settings - Fork 13
task: optimize vector indexes for brujula investigate index rebuild core 104 #690
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
Changes from all commits
b8f79b7
1366934
bf424f6
2943fc2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -388,51 +388,48 @@ async def _find_skills_of_occupation(self, occupation: OccupationEntity): | |
| if cached_result is not None: | ||
| return cached_result | ||
|
|
||
| skills = await self.relations_collection.aggregate([ | ||
| pipeline = [ | ||
| {"$match": {"modelId": self._model_id, "requiringOccupationId": ObjectId(occupation.id)}}, | ||
| {"$project": { | ||
| "requiredSkillId": 1, | ||
| "modelId": 1, | ||
| "relationType": 1, | ||
| "signallingValueLabel": 1, | ||
| }}, | ||
| { | ||
| "$lookup": { | ||
| "from": self.embedding_config.skill_collection_name, | ||
| "localField": "requiredSkillId", | ||
| "foreignField": "skillId", | ||
| "as": "skills", | ||
| "pipeline": [ | ||
| {"$match": {"modelId": self._model_id}} | ||
| {"$match": {"modelId": self._model_id}}, | ||
| # Limit 1 of the skills (Note: each skill has 3 corresponding documents, pick the first. | ||
| {"$limit": 1}, | ||
| # Exclude large/irrelevant fields from the result to reduce network transfer and improve latency | ||
| {"$project": { "embedding": 0, "embedded_field": 0, "embedded_text": 0, }} | ||
| ] | ||
| } | ||
| }, | ||
| {"$unwind": "$skills"}, | ||
| {"$group": {"_id": "$skills.skillId", | ||
| "modelId": {"$first": "$modelId"}, | ||
| "skillId": {"$first": "$skills.skillId"}, | ||
| "UUID": {"$first": "$skills.UUID"}, | ||
| "preferredLabel": {"$first": "$skills.preferredLabel"}, | ||
| "description": {"$first": "$skills.description"}, | ||
| "scopeNote": {"$first": "$skills.scopeNote"}, | ||
| "originUUID": {"$first": "$skills.originUUID"}, | ||
| "UUIDHistory": {"$first": "$skills.UUIDHistory"}, | ||
| "altLabels": {"$first": "$skills.altLabels"}, | ||
| "skillType": {"$first": "$skills.skillType"}, | ||
| "relationType": {"$first": "$relationType"}, | ||
| "signallingValueLabel": {"$first": "$signallingValueLabel"}, | ||
| } | ||
| } | ||
| ]).to_list(length=None) | ||
| {"$unwind": "$skills"} | ||
| ] | ||
|
|
||
| skills_relationships = await self.relations_collection.aggregate(pipeline).to_list(length=None) | ||
| result = [AssociatedSkillEntity( | ||
| id=str(skill.get("skillId", "")), | ||
| modelId=str(skill.get("modelId", "")), | ||
| UUID=skill.get("UUID", ""), | ||
| preferredLabel=skill.get("preferredLabel", ""), | ||
| description=skill.get("description", ""), | ||
| scopeNote=skill.get("scopeNote", ""), | ||
| altLabels=skill.get("altLabels", []), | ||
| skillType=skill.get("skillType", ""), | ||
| originUUID=skill.get("originUUID", ""), | ||
| UUIDHistory=skill.get("UUIDHistory", []), | ||
| relationType=skill.get("relationType", ""), | ||
| signallingValueLabel=skill.get("signallingValueLabel", ""), | ||
| id=str(skill_relationship.get("skills").get("skillId", "")), | ||
| modelId=str(skill_relationship.get("modelId", "")), | ||
| UUID=skill_relationship.get("skills").get("UUID", ""), | ||
| preferredLabel=skill_relationship.get("skills").get("preferredLabel", ""), | ||
| description=skill_relationship.get("skills").get("description", ""), | ||
| scopeNote=skill_relationship.get("skills").get("scopeNote", ""), | ||
| altLabels=skill_relationship.get("skills").get("altLabels", []), | ||
| skillType=skill_relationship.get("skills").get("skillType", ""), | ||
| originUUID=skill_relationship.get("skills").get("originUUID", ""), | ||
| UUIDHistory=skill_relationship.get("skills").get("UUIDHistory", []), | ||
| relationType=skill_relationship.get("relationType", ""), | ||
| signallingValueLabel=skill_relationship.get("signallingValueLabel", ""), | ||
| score=0.0 | ||
| ) for skill in skills] | ||
| ) for skill_relationship in skills_relationships] | ||
|
Comment on lines
+422
to
+432
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bug: The removal of the Suggested FixReintroduce a deduplication mechanism. The safest approach is to add a Prompt for AI AgentDid we get this right? 👍 / 👎 to inform future reviews.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @sentry, the fact that I added limit 1 in the previous stage pipeline, won't that sovle the problem of removing duplicates? |
||
|
|
||
| # Cache the result | ||
| await _skills_of_occupation_cache.set(occupation.id, result) | ||
|
|
||
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.
The
$limit: 1stage in the inner$lookuppipeline picks an arbitrary document (the first one returned by MongoDB) out of the potentially 3 copies of a skill document (one per embedded field: description, preferredLabel, altLabels). Because there is no$sortbefore the$limit, the document returned is non-deterministic and may vary between queries or after index changes. While the non-embedding fields should be identical across all 3 copies, it would be safer and clearer to add a$sort: {"embedded_field": 1}(or any stable sort) before$limit: 1to ensure a deterministic and predictable result.