fix: address 5 code review findings (round 7)

- diversity: revert conf_array default from 0.5 back to 1.0 (np.ones);
  the 0.5 default caused None-confidence images to receive a 1.7× FPS
  boost and beat high-confidence detections — counter-productive for
  Frigate training data quality
- diversity: fix hard_count to include None-confidence images (count
  images where score is None or < 0.85, not only confirmed < 0.85);
  the previous check systematically undercounted boosted images when the
  Immich faces API omits the score field
- executor: fix garbled comment fragment "Skipped on / skipped when"
  left by a partial edit in round 4; merge into a single coherent sentence
- executor: expand actually_uploaded trade-off comment to document all
  three consequences of a tracker write failure (Frigate duplicate,
  quality-replacement exclusion, cap-slot consumption), not only the
  duplicate risk mentioned previously
- upload_tracker: add person_ids guard to reset_person isinstance check
  so the non-list warning only fires when cleanup would actually have run,
  not on no-op calls where person_ids is empty
This commit is contained in:
2026-06-16 23:09:14 +00:00
parent 692d77ee9f
commit 7282c76b68
3 changed files with 16 additions and 13 deletions
+5 -5
View File
@@ -517,9 +517,10 @@ def _cluster_aware_selection(
emb_normed = emb_matrix / np.maximum(norms, 1e-8) emb_normed = emb_matrix / np.maximum(norms, 1e-8)
# Build confidence weight array for hard example boosting. # Build confidence weight array for hard example boosting.
# Default to 0.5 for faces with no confidence score so they receive a # Default to 1.0 for faces with no confidence score: treat as high-confidence
# moderate diversity boost rather than being treated as high-confidence. # (no boost) rather than hard-example territory. A missing score field should
conf_array = np.full(n, 0.5) # not cause these images to beat genuinely high-confidence detections in FPS.
conf_array = np.ones(n)
if confidence_scores: if confidence_scores:
for i, c in enumerate(confidence_scores): for i, c in enumerate(confidence_scores):
if c is not None: if c is not None:
@@ -586,8 +587,7 @@ def _cluster_aware_selection(
1 for i in selected 1 for i in selected
if confidence_scores if confidence_scores
and i < len(confidence_scores) and i < len(confidence_scores)
and confidence_scores[i] is not None and (confidence_scores[i] is None or confidence_scores[i] < 0.85)
and confidence_scores[i] < 0.85
) )
logger.info("Selection complete: %s images (%s hard examples with confidence < 0.85).", len(selected), hard_count) logger.info("Selection complete: %s images (%s hard examples with confidence < 0.85).", len(selected), hard_count)
+10 -7
View File
@@ -396,8 +396,8 @@ def upload_to_frigate(jobs: list[dict]) -> None:
# Pre-upload Frigate score — clean measurement (image not yet in training set). # Pre-upload Frigate score — clean measurement (image not yet in training set).
# Called for all below-cap uploads (seeds frigate_scores for future at-cap # Called for all below-cap uploads (seeds frigate_scores for future at-cap
# replacement) and for at-cap uploads when scores already exist. Skipped on # replacement) and for at-cap uploads when scores already exist.
# skipped when has_frigate_model is False (effective_count was 0 before the loop). # Skipped when has_frigate_model is False (effective_count was 0 before the loop).
# recognize_face returns (face_name, score); we only use the score when the # recognize_face returns (face_name, score); we only use the score when the
# best match is for the correct person. Mismatches (or "unknown") are treated # best match is for the correct person. Mismatches (or "unknown") are treated
# as None so a wrong-person score never drives a ceiling skip or replacement. # as None so a wrong-person score never drives a ceiling skip or replacement.
@@ -539,11 +539,14 @@ def upload_to_frigate(jobs: list[dict]) -> None:
person_has_fscores = True person_has_fscores = True
# Always record for reconcile so the Frigate filename→asset_id # Always record for reconcile so the Frigate filename→asset_id
# mapping is created even when the tracker write fails. # mapping is created even when the tracker write fails.
# Trade-off: if mark_uploaded failed, this asset_id is not in # Trade-off: if mark_uploaded failed, asset_id is absent from
# the upload set, so it may be re-selected next run (Frigate # asset_ids and scores. Consequences: (1) re-selected next run
# duplicate). The alternative — not appending — leaves the file # → Frigate duplicate; (2) excluded from quality-replacement
# permanently unmapped, breaking quality-replacement scoring. # candidates (_pick_mapped_file requires a scores entry);
# Frigate duplicate is the lesser consequence. # (3) counted toward MAX_AUTO_IMAGES cap (via frigate_files).
# The alternative — not appending — leaves the file permanently
# unmapped (reconcile never creates the frigate_files entry),
# making (2) and (3) permanent. Frigate duplicate is lesser.
actually_uploaded.append((fname, asset_id)) actually_uploaded.append((fname, asset_id))
break break
+1 -1
View File
@@ -442,7 +442,7 @@ def reset_person(person_name: str) -> None:
data["by_person"] = by_person data["by_person"] = by_person
flat_key = _flat_key(filename) flat_key = _flat_key(filename)
person_ids = set(_get_ids(tracker_entry)) person_ids = set(_get_ids(tracker_entry))
if flat_key in data and not isinstance(data[flat_key], list): if person_ids and flat_key in data and not isinstance(data[flat_key], list):
logger.warning( logger.warning(
"reset_person: %s has unexpected type for %s (%s) — skipping flat-list cleanup", "reset_person: %s has unexpected type for %s (%s) — skipping flat-list cleanup",
filename, flat_key, type(data[flat_key]).__name__, filename, flat_key, type(data[flat_key]).__name__,