Repository navigation
fix(sync): join a landed sync clip to the live episode at intake, not after notes - #23563
Conversation
… after notes Offline-sync (WAL) clips rendered on Home as standalone rows with raw first-line titles until enrichment finalized and _attach_synced finally joined them to the live episode. The join now runs the moment intake persisted the row (complete_sync_intake -> finalize.attach_synced_at_intake -> service.attach_synced_intake), with the same cheap rules: no model call, never opens or closes an episode, joins only the current live episode covering the span. The finalize-time sync_update join stays as an idempotent safety net. A clip that starts before the episode's recorded start now joins when its span reaches the episode or it began within CONTINUE_GAP_SECONDS of that start (pendant and phone clock separately); one that ended earlier belongs to an occasion already over and stays an ordinary row. Smart-merge donors already repoint membership to the survivor and intake absorption tombstones donors deleted+discarded, so Home never shows a dead donor; a test covers the survivor repoint for an intake-joined clip. Failure-Class: FC-episode-membership-waits-for-enrichment Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2f9298d523
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| owner = episodes_db.get_episode_for_conversation(uid, conversation_id, firestore_client=c.firestore_client) | ||
| if owner is not None: | ||
| return 'attached' if policy.is_live(owner) else 'no_episode' | ||
| return _attach_synced(uid, conversation_id, row, c) |
There was a problem hiding this comment.
When a WAL clip joins before the mobile occasion conversation finishes, _attach_synced appends its ID to conversation_ids. However, _close_for_finalize treats ids[-1] != conversation_id as proof that another capture is continuing, so a subsequent capture_end or server_recovery for the mobile conversation returns continued instead of closing the episode. The sync member cannot close it either because sync_update never closes episodes. This newly affects clips whose enrichment is still pending when capture ends: the ended occasion remains live until idle cleanup and can absorb subsequent speech. Exclude parallel sync membership from the ordering used to decide whether the live capture has ended.
Useful? React with 👍 / 👎.
… off the sync hot path Review follow-ups on intake attach: a discarded fragment or merge donor never joins; a member later folded by sync intake follows its survivor via sync_merged_from; the join runs on the episode pool so sync intake never waits on episode Firestore reads (finalize-time join stays the safety net). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e18f0b3de5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| except Exception as error: | ||
| logger.warning('event=episode_sync_intake_attach outcome=error exception_type=%s', type(error).__name__) |
There was a problem hiding this comment.
When the episode pool rejects submission, this handler logs a warning and lets sync continue without the intake fold, but never records the degraded outcome through utils.observability.fallback.record_fallback. The new exception handlers in attach_synced_at_intake and service.attach_synced_intake have the same omission. Consequently, failed intake attachment can restore the minutes-late standalone Home rows without appearing in the shared fallback metrics. Instrument these new fail-open paths with the shared helper and outcome='degraded', as required by the backend guide.
AGENTS.md reference: backend/AGENTS.md:L316-L316
Useful? React with 👍 / 👎.
| if row.get('discarded') or row.get('deleted') or row.get('sync_merged_into'): | ||
| # A discarded fragment or a merge donor never joins (the finalize path's rule). | ||
| return 'skipped_discarded' | ||
| _repoint_sync_donors(uid, conversation_id, row, c) |
There was a problem hiding this comment.
When a later sync intake absorbs a clip before its queued episode callback executes, row still contains the pre-merge snapshot copied by schedule_synced_intake_attach, so this check misses the persisted deleted/sync_merged_into tombstone. The episode executor has multiple workers, allowing the survivor callback to run first, find no donor membership to repoint, and attach the survivor; the delayed donor callback then appends the deleted donor. Neither sync finalization nor bridge cleanup subsequently removes that membership. This leaves a dead conversation in the episode, and PreviousEpisodePage renders an Open Conversation button for it that cannot load the deleted row. Revalidate the persisted deletion/redirect state when executing the attachment rather than relying solely on the queued snapshot.
Useful? React with 👍 / 👎.
Problem
While a live episode was on screen ("Listening now · Live 10m"), pendant offline-sync clips (WAL,
capture_evidence.origin=sync_vad, finalization triggersync_update) rendered on Home as standalone rows with raw first-line titles ("Mm, your phone." 19s, "Check." 7s, …) for the 5–10 minutes until enrichment finished and the server merged/titled them. David's expectation: clips of the ongoing occasion are part of the live episode and should never appear as separate junk rows.Root cause
_attach_synced(called only fromon_conversation_finalized) was the only place a sync conversation joined an episode, and it runs after the clip's normal processing completes. Until then the clip is a standalone conversation and Home renders it. Additionally,if episode_started is not None and started < episode_started: return 'no_episode'permanently orphaned a clip that starts slightly before the live episode's recorded start (the pendant and the phone clock separately, and the episode's recorded start can sit slightly after the speech).Change
Attach at intake — the moment a sync clip row is persisted (which is when Home can first see it):
utils/episodes/service.pyattach_synced_intake: joins the landed row to the current live episode via the same_attach_syncedrules — no model call, never opens or closes an episode, joins only the current live episode covering its span, and a conversation already owned by an episode (live or closed) never moves. Fail-open, never raises.utils/episodes/finalize.pyattach_synced_at_intake: the sync-lane hook (gating + deferred service import + pydantic-enumsourcenormalization, mirroringclose_now).utils/sync/intake_completion.pycomplete_sync_intake/utils/sync/pipeline.pyprocess_segment: the hook runs right after intake enrollment, before audio storage or enrichment can stall behind it, whenever the intake wrote content (not for transient repeat-only intakes).Tolerance —
_attach_syncedno longer hard-rejects a clip that started before the episode's recorded start. It joins when the clip's span reaches the episode (a parallel capture that caught the opening words) or it began withinCONTINUE_GAP_SECONDS(120 s) of the episode's start (clock skew). A clip that ended earlier than that belongs to an occasion already over and stays an ordinary row. The boundary rules (ambiguous gap, different calendar event, hard cap) still gate every join.Idempotent finalize — the finalize-time
sync_updatejoin stays as the safety net for rows that landed before intake could attach (pre-deploy rows, a failed intake attach, or an episode that opened only after the clip landed); with membership already present it is a no-op ('finalized'), feeding no duplicate text and making no model call.Merges (verified, no new machinery needed) — smart-merge donors already repoint episode membership to the survivor (
_repoint_absorbedvia_episode_after_absorb), and intake-assignment absorption (sync_merged_into) tombstones donors asdeleted+discarded, so Home can never render a dead donor. A test covers an intake-joined clip whose membership follows to the survivor.Mobile Home fold (verified, no app change) —
hiddenLiveConversations(episodes.liveConversationIds)inapp/lib/pages/conversations/live_episode_visibility.darthides episode members under the live row as soon as membership exists in the provider,EpisodeProviderpollsrefreshLive()every 30 s while the live row is on screen (LiveEpisodeRow.watch()), and pull-to-refresh refreshes it too. With membership now existing within ~a second of the row, the fold happens on the next poll instead of after enrichment.Gating — the intake attach respects
EPISODES_ENABLED/episodes_enabled_for(uid)and theclient_gatecapability marker (client_seen_cached, so a faulting Redis costs one read per 120 s window instead of one per segment).EPISODES_CLIENT_CHANNELSand env defaults are untouched.Docs:
backend/utils/episodes/ARCHITECTURE.mdand the service/finalize module docstrings describe the new boundary.Tests
New/updated backend unit tests:
'attached'(test_episodes_service.py)'disabled', no capability marker'skipped_no_client', live-episode member'attached', closed-episode owner'no_episode', no live episode creates nothingCONTINUE_GAP_SECONDS(ended just before the recorded start) joins, ended earlier than that stays outsource→ wire value) and fail-open (test_episodes_finalize_paths.py)complete_sync_intakecalls the attach before audio/enrichment, skips repeat-only intakes;process_segmenthands the persisted row to the hook (test_sync_lineage_diagnostics.py,test_sync_capture_pipeline.py)Run (this worktree, per-file contract runner):
Failure-Class: new
Line-Count-Exception: backend/utils/sync/pipeline.py | 3755 -> 3756 | one intake kwarg hands the landed row to the episodes attach; no other line changes
🤖 Generated with Claude Code