Skip to content
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

优化人像搜索 #1211

Merged
merged 1 commit into from
Dec 26, 2024
Merged

优化人像搜索 #1211

merged 1 commit into from
Dec 26, 2024

Conversation

guochenshu
Copy link
Contributor

No description provided.

@guochenshu guochenshu enabled auto-merge (squash) December 26, 2024 09:08
Copy link

codecov bot commented Dec 26, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 30.27%. Comparing base (219e974) to head (8b45632).
Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1211   +/-   ##
=======================================
  Coverage   30.27%   30.27%           
=======================================
  Files          50       50           
  Lines        3121     3121           
=======================================
  Hits          945      945           
  Misses       2125     2125           
  Partials       51       51           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Summary

This PR adds comprehensive face tagging functionality and text search capabilities to the face recognition demo. Here are the key changes:

  • Added new ImagePreview.vue component for viewing and tagging detected faces with proper scaling, cropping and editing capabilities
  • Implemented automatic tag propagation system that applies tags to similar faces based on configurable similarity threshold (0.75)
  • Added SQLite database support for face tags with new methods for tag management and text-based face searching
  • Enhanced frontend with text search input and face tag visualization in both management and search views
  • Added proper error handling and loading states for tag operations and search functionality

The implementation is well-structured but should consider adding database migration code for existing installations.

💡 (1/5) You can manually trigger the bot by mentioning @greptileai in a comment!

6 file(s) reviewed, 12 comment(s)
Edit PR Review Bot Settings | Greptile

Comment on lines +136 to +140
// 应缩放比例到坐标
const x = face.face_coords.x1 * scaleX
const y = face.face_coords.y1 * scaleY
const w = (face.face_coords.x2 - face.face_coords.x1) * scaleX
const h = (face.face_coords.y2 - face.face_coords.y1) * scaleY
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logic: Missing null check for face_coords before accessing x1/y1/x2/y2 properties. Could cause runtime errors if face detection fails.

inputPlaceholder: '请输入标签名称'
})

if (tag !== null) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logic: tag !== null check will not catch empty strings. Should validate tag has meaningful content before making API call.

Comment on lines +200 to +206
const response = await fetch('/api/face/tag', {
method: 'POST',
headers: {
'Content-Type': 'application/json',
},
body: JSON.stringify(requestData)
})
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: API call lacks error handling for network failures and timeout scenarios

Comment on lines +217 to +219
if (error !== 'cancel') {
ElMessage.error(error.message || '标签更新失败')
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: Error handling only checks for 'cancel' string, but ElMessageBox.prompt can reject with various values. Consider more robust error type checking.

@@ -282,7 +283,7 @@ def delete_face_vectors(self, photo_id: int) -> bool:
# # 执行压缩操作以回收空间
# self.collection.compact()

logger.info(f"成功删除图片ID {photo_id} 的人脸向量")
logger.info(f"成功删除片ID {photo_id} 的人脸向量")
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

syntax: typo in log message: '片ID' should be '图片ID'

Comment on lines +111 to +116
similar_faces = db_manager.search_similar_faces(process_result[1]['embeddings'][i].tolist())
if similar_faces:
# 找出相似度最大的人脸
max_similarity_face = max(similar_faces, key=lambda face: face['similarity'])
if max_similarity_face['similarity'] > SIMILARITY_FOR_TAG:
tag = db_manager.get_face_tag(max_similarity_face['id'], max_similarity_face['face_index'])
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logic: searching for similar faces before inserting the new face vectors could miss potential matches since the current face isn't in the database yet

Comment on lines +343 to +350
if similar_faces:
for face in similar_faces:
if db_manager.update_face_tag(face['id'], face['face_index'], request.tag):
updated_count += 1

# 确保当前选中的人脸也被标记
if db_manager.update_face_tag(request.image_id, request.face_index, request.tag):
updated_count += 1
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: no transaction wrapping the multiple tag updates - could leave database in inconsistent state if operation fails partway through

Comment on lines +275 to +280
const handleTagViewerClose = () => {
showTagViewer.value = false
showViewer.value = false
previewUrl.value = ''
currentFaces.value = []
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logic: currentImage not being cleared in handleTagViewerClose

.map(item => ({
url: item.oss_url,
filename: item.original_filename,
similarity: Math.max(...item.faces.map(face => face.similarity)),
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logic: same Math.max() issue with empty faces array as in image search

Comment on lines +270 to +272
const response = await axios.post('/api/text/search', {
query: searchText.value.trim()
})
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: consider adding rate limiting or debouncing for text search to prevent API abuse

@guochenshu guochenshu merged commit be1e484 into main Dec 26, 2024
6 checks passed
@guochenshu guochenshu deleted the feat/face-local-model branch December 26, 2024 09:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants