Skip to content

Commit a0c75db

Browse files
committed
fix(screenpipe): keep the chosen frame as timeline evidence through a discard
A discarded observation had its media cleared, so the frame the agent had just chosen was thrown away. Discard means the vault needs no note about the observation — not that the day's visual timeline should have a blank where it happened. media_data is what evidence.py turns into an evidence item's image_filename, which is where an episode's representative_image comes from, so clearing it is what left gaming sessions with no picture anywhere. The selection now applies on every branch; discard and duplicate clear the media paths only when no frame was chosen. Putting an image inside a note stays a separate decision behind promote_image/retain_image, which most observations should not take. The rest of the shortlist is dropped once it has been chosen from — it exists to be judged, and 2-6 frames per observation is several times the storage of the one that was picked.
1 parent 450cb4c commit a0c75db

3 files changed

Lines changed: 88 additions & 45 deletions

File tree

backends/advanced/src/advanced_omi_backend/services/observation_curation.py

Lines changed: 41 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -106,10 +106,11 @@ def _observation_codex_settings(settings: Any = None) -> dict[str, Any]:
106106
107107
Images named `frame-<id>.jpg` are a shortlist sampled across this observation's span,
108108
not a sequence to describe. Read them for what the session actually was — the game or
109-
document on screen, what was being done — and set `selected_frame_id` to the one that
110-
best represents it, or null if none is worth keeping. Prefer clearly legible content
111-
over an exact moment; "good enough" is the bar. Frames are also evidence you may cite
112-
in the summary, not only thumbnail choices.
109+
document on screen, what was being done — and set `selected_frame_id` to the frame that
110+
best depicts it. This is the observation's thumbnail on the visual timeline and is kept
111+
whatever you decide below, so choose one even when the observation itself is routine and
112+
you discard it; use null only when no frame depicts anything. Prefer clearly legible
113+
content over an exact moment; "good enough" is the bar.
113114
114115
The screenshot is sparse supporting evidence, not permission to invent. `output` audio
115116
is system/media audio: character dialogue, lyrics, presenters, and game dialogue are
@@ -120,7 +121,8 @@ def _observation_codex_settings(settings: Any = None) -> dict[str, Any]:
120121
Use `text_update` for a small Daily/YYYY-MM-DD.md entry. Use `dedicated_note` only for a
121122
durable event/project/topic/place/media experience; choose a safe relative `.md` path.
122123
Use `promote_image` only when the ScreenPipe preview or one of the supplied Immich
123-
thumbnail candidates adds durable value; set `immich_item_id` only for the latter. Use `duplicate` only when the supplied canonical observation id is clear.
124+
thumbnail candidates adds durable value to the note itself; set `immich_item_id` only
125+
for the latter. Use `duplicate` only when the supplied canonical observation id is clear.
124126
Never write a fake conversation note for screen context.
125127
"""
126128

@@ -598,30 +600,43 @@ async def apply_curation_decision(
598600
action = decision["decision"]
599601

600602
updates: dict[str, Any] = {"agent_reason": agent_reason}
601-
# The frame the agent chose becomes the observation's representative image, so
602-
# every downstream consumer — vault promotion, the 1280px source-media fetch, the
603-
# timeline thumbnail — uses the frame that was actually judged to depict this
604-
# session, not the one that scored highest before anyone looked at it.
603+
# The frame the agent chose becomes the observation's representative image. It is
604+
# kept whatever the decision below is, because it is *timeline* evidence, not vault
605+
# content: discarding an observation means the vault does not need a note about it,
606+
# not that the day's visual timeline should have a blank where it happened. Only
607+
# `promote_image`/`retain_image` puts an image inside a note.
605608
#
606-
# Applied in memory here because the promotion path below reads `item.media_data`,
607-
# but written only on the branches that keep an image: discard and duplicate
608-
# `$unset` these same paths, and Mongo rejects a `$set` and `$unset` of one path
609-
# in a single update.
609+
# The shortlist itself is dropped once it has been chosen from — it exists to be
610+
# judged, and keeping 2-6 frames per observation is several times the storage of
611+
# the one that was picked.
610612
selected = _selected_preview(item, decision)
611-
selection_updates: dict[str, Any] = {}
612613
if selected is not None:
613614
item.media_data = selected["data"]
614615
item.media_content_type = selected["content_type"]
615616
item.media_filename = f"frame-{selected['frame_id']}.jpg"
616617
item.content_hash = hashlib.sha256(selected["data"]).hexdigest()
617-
selection_updates = {
618-
"media_data": item.media_data,
619-
"media_filename": item.media_filename,
620-
"media_content_type": item.media_content_type,
621-
"content_hash": item.content_hash,
622-
"metadata.preview_frame_id": selected["frame_id"],
623-
"metadata.thumbnail_available": True,
624-
}
618+
updates.update(
619+
{
620+
"media_data": item.media_data,
621+
"media_filename": item.media_filename,
622+
"media_content_type": item.media_content_type,
623+
"content_hash": item.content_hash,
624+
"metadata.preview_frame_id": selected["frame_id"],
625+
"metadata.thumbnail_available": True,
626+
}
627+
)
628+
if item.media_previews:
629+
updates["media_previews"] = []
630+
item.media_previews = []
631+
# Paths the discard/duplicate branches clear when no thumbnail was chosen. Mongo
632+
# rejects a `$set` and `$unset` of the same path in one update, so a chosen frame
633+
# takes precedence and nothing is unset.
634+
_MEDIA_PATHS = (
635+
"media_data",
636+
"media_filename",
637+
"media_content_type",
638+
"content_hash",
639+
)
625640
unset: tuple[str, ...] = ()
626641
if action == "duplicate":
627642
target_id = decision.get("duplicate_observation_id")
@@ -638,24 +653,13 @@ async def apply_curation_decision(
638653
raise ValueError("agent selected an invalid duplicate observation")
639654
updates["duplicate_of"] = str(target.id)
640655
updates["curation"] = "duplicate"
641-
if item.lifecycle == "closed":
642-
unset = (
643-
"media_data",
644-
"media_filename",
645-
"media_content_type",
646-
"content_hash",
647-
)
656+
if item.lifecycle == "closed" and selected is None:
657+
unset = _MEDIA_PATHS
648658
elif action == "discard":
649659
updates["curation"] = "discarded"
650-
if item.lifecycle == "closed":
651-
unset = (
652-
"media_data",
653-
"media_filename",
654-
"media_content_type",
655-
"content_hash",
656-
)
660+
if item.lifecycle == "closed" and selected is None:
661+
unset = _MEDIA_PATHS
657662
else:
658-
updates.update(selection_updates)
659663
retain_image = bool(decision.get("retain_image")) or action == "promote_image"
660664
immich_item_id = decision.get("immich_item_id") if retain_image else None
661665
if (

backends/advanced/tests/test_observation_curation.py

Lines changed: 34 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -326,12 +326,12 @@ def test_only_a_shortlisted_frame_can_be_selected():
326326

327327

328328
@pytest.mark.asyncio
329-
async def test_discarding_a_selected_frame_does_not_set_and_unset_one_path(monkeypatch):
330-
"""A discarded observation drops its image; it must not also write one.
329+
async def test_discarded_observation_keeps_the_chosen_timeline_thumbnail(monkeypatch):
330+
"""Discard means "the vault needs no note", not "the timeline needs a blank".
331331
332-
Mongo rejects a `$set` and `$unset` of the same path in one update, so applying
333-
the agent's frame choice unconditionally made every discard of a closed
334-
observation fail with "Updating the path 'media_data' would create a conflict".
332+
The chosen frame is timeline evidence, so it survives a discard. It must also not
333+
be written and cleared in one update: Mongo rejects a `$set` and `$unset` of the
334+
same path, which previously failed every discard of a closed observation.
335335
"""
336336

337337
item = observation(
@@ -355,6 +355,35 @@ async def capture(target, fields, *, unset=()):
355355
observation_revision(item),
356356
)
357357

358+
fields, unset = seen[-1]
359+
assert fields["curation"] == "discarded"
360+
assert fields["media_data"] == b"jpeg"
361+
assert fields["metadata.preview_frame_id"] == 7
362+
assert not unset
363+
assert not set(fields) & set(unset)
364+
# The shortlist is spent once it has been chosen from.
365+
assert fields["media_previews"] == []
366+
367+
368+
@pytest.mark.asyncio
369+
async def test_discard_without_a_chosen_frame_still_clears_media(monkeypatch):
370+
"""Nothing depicted the observation, so there is no thumbnail worth storing."""
371+
372+
item = observation(lifecycle="closed", curation="curating")
373+
seen: list[tuple[dict, tuple]] = []
374+
375+
async def capture(target, fields, *, unset=()):
376+
seen.append((fields, unset))
377+
return True
378+
379+
monkeypatch.setattr(observation_curation, "_apply_curation_fields", capture)
380+
381+
await apply_curation_decision(
382+
item,
383+
{"decision": "discard", "reason": "Routine", "selected_frame_id": None},
384+
observation_revision(item),
385+
)
386+
358387
fields, unset = seen[-1]
359388
assert "media_data" in unset
360389
assert not set(fields) & set(unset)

docs/screenpipe.md

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -170,9 +170,19 @@ its store, so a missing frame is expected, does not fail the batch, and is recor
170170

171171
The curation agent is then shown every fetched frame and picks the one that represents
172172
the session (`selected_frame_id`), or none. That selection — not the scorer's guess —
173-
becomes the observation's `media_data`, and the image it chooses is fetched at a bounded
174-
1280px only if it is promoted into the vault. ScreenPipe remains the high-resolution
175-
source and Chronicle never uploads a frame sequence.
173+
becomes the observation's `media_data`, and the rest of the shortlist is dropped: it
174+
existed to be judged, and storing every frame costs several times the one that was
175+
picked. ScreenPipe remains the high-resolution source and Chronicle never uploads a
176+
frame sequence.
177+
178+
**A chosen frame is timeline evidence, so it survives a `discard`.** Discarding means
179+
the vault needs no note about the observation, not that the day's visual timeline should
180+
have a blank where it happened — `services/timeline/evidence.py` turns `media_data` into
181+
an evidence item's `image_filename`, which is what an episode's `representative_image`
182+
is drawn from. Only `promote_image`/`retain_image` puts an image *inside* a note, and
183+
that path additionally fetches the frame at a bounded 1280px and content-addresses it
184+
under `_media/`. The two decisions are independent: most observations should yield a
185+
timeline thumbnail and no vault image.
176186

177187
The request is bounded: a shortlist is asked for at most twice, after which curation
178188
proceeds on text alone. It must be, because a frame ScreenPipe has pruned returns 404

0 commit comments

Comments
 (0)