Skip to content

fix(sync): fence post-merge audio finalization#10274

Merged
kodjima33 merged 3 commits into
BasedHardware:mainfrom
Git-on-my-level:fix/sync-fenced-finalization-10234
Jul 22, 2026
Merged

fix(sync): fence post-merge audio finalization#10274
kodjima33 merged 3 commits into
BasedHardware:mainfrom
Git-on-my-level:fix/sync-fenced-finalization-10234

Conversation

@Git-on-my-level

@Git-on-my-level Git-on-my-level commented Jul 22, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Exclude a lifecycle-fenced merged conversation from updated_memories after deferred reprocessing loses ownership.
  • Preserve processing and audio finalization for unaffected sibling conversations.
  • Prevent the later private-cloud audio finalizer from writing audio_files, precache state, or an artifact job for a fenced conversation.

Failure class

Failure-Class: FC-split-mutation-authority

Verification

  • BACKEND_UNIT_TEST_FILE_LIST=/tmp/omi-10234-fenced-finalization-tests.txt bash backend/test.sh -- -k merged_reprocess_fence_isolated_to_its_conversation -vv — 164 passed.
  • The regression first failed before the production change: the fenced ID remained in updated_memories; after the fix it proves only the current sibling reaches audio finalization.

Risk

Low. The change removes post-fence side effects only for the conversation whose lifecycle ownership was explicitly lost. It does not change the fence itself, retry/terminal semantics, or the sibling-success path.

Closes #10234

Review in cubic

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a9bdf953ec

ℹ️ 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".

Comment thread backend/utils/sync/pipeline.py
@Git-on-my-level
Git-on-my-level force-pushed the fix/sync-fenced-finalization-10234 branch 5 times, most recently from 9022f25 to 1082f6f Compare July 22, 2026 07:58
@Git-on-my-level

Copy link
Copy Markdown
Collaborator Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1082f6f9c6

ℹ️ 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".

Comment thread backend/utils/sync/pipeline.py
@Git-on-my-level
Git-on-my-level force-pushed the fix/sync-fenced-finalization-10234 branch 2 times, most recently from e5a3c6f to 8722593 Compare July 22, 2026 11:04
Failure-Class: FC-split-mutation-authority
@Git-on-my-level
Git-on-my-level force-pushed the fix/sync-fenced-finalization-10234 branch from 8722593 to 9189564 Compare July 22, 2026 11:11
@Git-on-my-level

Copy link
Copy Markdown
Collaborator Author

Addressed Codex review feedback (discussion_r3628593415): a fenced conversation ID that appeared in both new_memories and updated_memories was only discarded from updated_memories, leaving it in new_memories. Since _finalize_sync_audio_files unions both buckets, the fenced conversation still received audio/precache/artifact side effects.

Fix: discard fenced IDs from new_memories in both the fence handler (_reprocess_merged_conversations) and the retry-hydration path. Updated the unit tests to cover the dual-bucket scenario and the retry path.

Also repaired two rebase casualties: restored reportUnusedImport=false on the pipeline pyright comment and updated the e2e stub for the new on_fenced parameter.

A conversation created in an early segment (new_memories) can be merged
into by a later segment (updated_memories). When deferred reprocessing
raises SyncConversationPersistenceFenced, the fence handler only
discarded the ID from updated_memories. _finalize_sync_audio_files
unions new_memories | updated_memories, so it would still write audio
files to the fenced conversation.

Also applies the same subtraction in retry rehydration, which only
subtracted fenced IDs from updated_memories.
@Git-on-my-level

Copy link
Copy Markdown
Collaborator Author

Monitor pass: resolved Codex thread on _reprocess_merged_conversations fence persistence (discussion_r3628072595). The fence is already durably checkpointed to both the content ledger and job partial result via _checkpoint_fenced_conversations_for_run before audio finalization, and retry hydration unions the tombstone and filters stale IDs from updated_memories/new_memories — the concern was fully addressed in the existing code.

Backend unit suite remains red, but the 3 failing files (test_desktop_transcribe.py, test_smoke_what_matters_now.py, test_workflow_contracts.py) fail identically on upstream main (571ece3) and are unrelated to this PR's sync pipeline changes.

@kodjima33 kodjima33 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

fix(sync): fence post-merge audio finalization, closes #10234, regression test proves fix, CI green — 5/5

@kodjima33
kodjima33 merged commit de20069 into BasedHardware:main Jul 22, 2026
26 of 27 checks passed
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.

Reconcile superseded offline-sync jobs without stale-worker retries

2 participants