[ocakciomursel-pixel]

Fixes #23341

Summary

• Listen finalization worker (backend/routers/conversation_finalization.py): Catch InvalidArgument document size-limit failures (classify_finalization_failure(...).conversation_at_size_limit) in run_listen_finalization_job, close the oversized in_progress conversation via lifecycle.close_oversized_in_progress_conversation, and dead-letter the durable job with failure_code='document_size_limit' instead of retrying indefinitely.
• Live rollover at ~768 KiB / 1,500 segments (backend/config/occasion_conversations.py, backend/routers/listen/conversations.py):
  • Gate the live ~768 KiB (OCCASION_MAX_DOCUMENT_BYTES) / 1,500-segment (OCCASION_MAX_TRANSCRIPT_SEGMENTS) rollover behind LIVE_CONVERSATION_SIZE_ROLLOVER_ENABLED (on by default unless set to 0/false/off/no, registered in config/feature-flags.yaml).
  • Estimate conversation document bytes incrementally across lifecycle ticks (estimate_live_conversation_bytes_incremental with per-controller _size_estimate_state) so each 5-second tick only sizes newly settled segments and the active tail segment instead of re-traversing the entire document.
• Tests: Add unit coverage in backend/tests/unit/test_oversized_in_progress_close.py and backend/tests/unit/test_listen_occasion_conversations.py.

Failure-Class: FC-permanent-write-rejection-retried-forever

[Git-on-my-level]

Verdict: the oversized-document terminal path is well designed and well tested, but this needs two things resolved before merge — a set of sync-pipeline deletions the description never mentions, and an actual test run on this head.

What I verified and liked:

• config/occasion_conversations.py — occasion_size_exceeded() checks the cheap segment-count cap before the byte estimate; the 256 KiB headroom below Firestore's 1 MiB ceiling is the right shape.
• utils/conversations/quiet_finalization.py — run() now classifies the document-size rejection and routes to _close_oversized(), which stops the quiet check for closed/missing/not_in_progress/owned. That breaks the HTTP 500 requeue loop from #23341.
• routers/conversation_finalization.py — _terminalize_oversized_finalization_job() turns a permanent 400 into close + dead_letter with failure_code='document_size_limit'; the fanout_status == 'completed' fast path avoids double-terminalizing a finished fanout.
• database/conversation_terminal_title.py — the fit_document_limit() fallback to {'status': ...} guarantees the shrinking status write commits at the ceiling. Note the size check now also applies when extras is empty and base_update carries more than status — beyond the docstring's "optional growth" story; please confirm every caller tolerates losing extra base fields at the ceiling (dead_letter_conversation_updates is the one covered by tests).
• utils/enrichment/{outbox,consumer}.py + summary_delivery.py — retiring the obligation (superseded / DISCARDED, row dropped via the size_limit_superseded guard in finish()) instead of retrying forever is the correct terminal outcome.
• Performance: estimate_firestore_document_bytes() now serializes the full conversation dict on arm/prepare/lifecycle ticks — acceptable for the rollover checks, just be aware it runs on hot paths.
• Cost: this strictly reduces spend (stops paid retry loops); no new external calls.

Blocking:

1. Undisclosed deletions in backend/utils/sync/pipeline.py: record_sync(...), annotate_identity(...), the covering_target(...) fallback, the legacy_audio_trim() condition, and the recording_session_id plumbing through process_segment() are all removed, but the PR description doesn't mention them and the oversized-document fix doesn't require them. If these writes are already dead on main, please state that in the description with evidence and move the removal to its own PR; if they're live, this silently drops sync-coverage/capture-evidence/identity metadata and changes recording-session routing. process_segment() also loses a keyword parameter, which can break any out-of-tree caller.
2. No test workflow has run on this head — only the AI reviewer shows on the checks. Please get the backend unit suites running on CI for this commit (at minimum the five test files this diff touches); the local "139 passed" run can't be verified by reviewers.

Non-blocking notes:

• database/conversation_quiet_checks.py — _update_row_if_fits() silently skips arming writes for rows at the ceiling; the terminal path in quiet_finalization.run() does close that loop, but a one-line comment pointing there would stop a future reader from re-adding an unconditional update.
• utils/conversations/summary_delivery.py — the new 'document_size_limit' refusal (64 KiB headroom) permanently skips summary generation for near-ceiling rows, and the in_progress→completed terminal close also drops the summary. Reasonable degraded modes, but users with very long conversations will silently get no summary — flagging so a maintainer confirms the trade-off is intended.

USER IMPACT: very long conversations stop failing invisibly — they now close and roll over into a fresh conversation instead of looping errors, but conversations near the storage limit will appear without a generated summary.

Requesting maintainer review beyond the two blockers: the silent-summary-loss trade-off near the size ceiling is a product/data-availability decision, and the pipeline.py removals need someone who knows whether sync coverage is still live to rule on them.

---

by AI on behalf of David — if you need David’s attention urgently, please @Git-on-my-level and escalate with need human response.

[ocakciomursel-pixel]

Thanks for the thorough review @Git-on-my-level! Both blockers and the inline comment are resolved in 680b55d:

1. �ackend/utils/sync/pipeline.py rebased cleanly onto current main: The branch was previously based on an older commit and had a merge conflict against main (which also prevented GitHub from creating
   efs/pull/23348/merge and triggering the on: pull_request test workflows). After rebasing onto main, �ackend/utils/sync/pipeline.py is strictly +9/-0 lines (adding only the document_size_limit guard inside _reprocess_merged_conversations), with all
   ecord_sync, �nnotate_identity, covering_target, legacy_audio_trim, and
   ecording_session_id logic intact.
2. PR test suite now running: With mergeable: true, all Backend unit shard 1..6, Backend emulator proofs, Formatting, and Hygiene checks are triggered on the PR.
3. Inline comment in �ackend/database/conversation_quiet_checks.py: Added the explanatory comment on _update_row_if_fits documenting why
   aw_update is unaffected when status is present in update_fields.
4. Coalesced with main's quiet-finalization outbox retry semantics: Kept main's quiet_finalization.py and outbox.py (outcome == 'held' on retryable post-processing failures) while preserving the terminal size-cap closure (ailed + error_code='document_size_limit') across process_conversation, conversations_mutations.py, conversations_Protected.py, listen/conversations.py, and conversation_finalization.py.

Dismissed as resolved on head 1f56628: backend/utils/sync/pipeline.py is now strictly additive (+9/-0) after the rebase onto current main, and the backend unit + hermetic merge suites ran green on this head. needs-maintainer-review stays for the remaining product decision on summary loss near the size ceiling.

[Git-on-my-level]

Since my last review: addressed — backend/utils/sync/pipeline.py is now strictly +9/-0 after the rebase (the undisclosed deletions are gone; what remains is the disclosed document_size_limit continue in _reprocess_merged_conversations), and CI has now run on this head (Backend unit suite and Backend Hermetic Merge Gate green) · still open — the product trade-off on silent summary loss near the size ceiling needs a maintainer call · newly introduced — nothing blocking.

Verdict: both blockers from my last review are resolved; this is now a clean, well-tested follow-on to #23353, which explicitly deferred occasion rollover and the enrichment consumer path to a separate change. One product decision remains before merge.

Verified on this head:

• config/occasion_conversations.py — occasion_size_exceeded() checks the cheap 1,500-segment cap before serializing for the byte estimate; the deferred import keeps this config leaf free of utils imports.
• routers/listen/conversations.py — the size gate is wired at every attach point (_resume_continuation, reconnect in create_new_in_progress_conversation, prepare, lifecycle_loop). lifecycle_loop now rolls over on size even when occasion mode is off — that is what prevents recurrence for non-occasion capture, and test_the_lifecycle_loop_rolls_a_size_capped_conversation_over_before_1mib (enabled=False) pins it.
• routers/conversation_finalization.py — _terminalize_oversized_finalization_job() marks completed fanouts done, otherwise best-effort-closes then dead-letters with failure_code='document_size_limit'; a close failure only logs and still dead-letters, so the job cannot requeue forever.
• database/conversation_quiet_checks.py — _update_row_if_fits() skips arming stamps at the ceiling, now with the comment pointing at quiet_finalization.run() that I asked for last round.
• summary_delivery.py + utils/enrichment/consumer.py — near-ceiling rows are refused/discarded instead of retried. One asymmetry worth a maintainer's eye: #23353's prod outbox path parks rows as held, while this consumer path returns DISCARDED/SUPERSEDED — held can resume if the document ever shrinks, discarded cannot.
• stt/outcomes.py, sync/failure_reason.py, sync/pipeline.py — contract-style InvalidArgument classification (type-name + message, consistent with the file's existing convention), non-retryable sync_persistence_failed fingerprint, and the coalesced-batch continue.
• utils/conversations/lifecycle.py — thin firestore_client pass-through, no behavior change.

Performance: the byte estimate serializes the full conversation dict on arm/prepare/lifecycle ticks — acceptable, but it is on hot paths. Cost: strictly reduces spend by stopping paid retry loops. Security: no new surfaces.

USER IMPACT: very long conversations stop failing invisibly — they now close and roll over into a fresh conversation before hitting the storage ceiling, but conversations near that ceiling will appear without a generated summary.

Still wanted before merge: (1) a maintainer ruling that summary-less near-ceiling conversations — and the held-vs-discard asymmetry above — are the intended degraded mode; (2) a unit test for the close-failure path in _terminalize_oversized_finalization_job (close raises, job still dead-letters) if it is not already covered elsewhere.

---

by AI on behalf of David — if you need David’s attention urgently, please @Git-on-my-level and escalate with need human response.

[ocakciomursel-pixel]

Thanks for the review @Git-on-my-level! Following up on both points:

1. outbox.run (held) vs. EnrichmentConsumer.process (Outcome.DISCARDED / ConsumerReason.SUPERSEDED):
   Yes, this difference is intentional due to the two settlement contracts:
   • utils.enrichment.outbox.run performs an in-line CAS mutation on the row and parks it dormant with held_reason='document_size_limit' (existing behavior in outbox.py).
   • EnrichmentConsumer.process settles through utils.workers.consumer.run_push and the Outbox protocol (database/outbox.py), which does not define a separate HELD outcome; returning Result(Outcome.DISCARDED, ConsumerReason.SUPERSEDED) (the same terminal settlement used for SyncConversationPersistenceFenced and unrecoverable SummaryRepairSuperseded) delegates to outbox.ack(claim, reason, now=now) so the obligation is retired cleanly without re-arming or retrying. Also, if close_oversized_in_progress_conversation transiently fails during _terminalize_oversized_finalization_job, the listen sweep / recovery path (sweep_stale_in_progress_conversations) still picks up and closes any straggling oversized in_progress row once quiet.
2. Unit test for _terminalize_oversized_finalization_job when close_oversized_in_progress_conversation raises:
   Added test_terminalize_oversized_finalization_job_dead_letters_when_close_raises in backend/tests/unit/test_oversized_in_progress_close.py (9973d2dccc), verifying that when close_oversized_in_progress_conversation raises, the warning is logged, final_attempt_failed is still invoked with failure_code='document_size_limit', and the handler returns HTTP 200 {'status': 'dead_letter'} (plus coverage for the fanout_status == 'completed' branch returning {'status': 'done'}).

[Git-on-my-level]

Since my last review: addressed — the last untested path in the terminalization flow is now covered: test_terminalize_oversized_finalization_job_dead_letters_when_close_raises (backend/tests/unit/test_oversized_in_progress_close.py) verifies that when close_oversized_in_progress_conversation raises inside backend/routers/conversation_finalization.py, the job still dead-letters with failure_code document_size_limit and returns 200, and that a completed fanout marks the job done; also verified the rebase onto current main left the 12-file diff intact (backend/utils/sync/pipeline.py remains strictly +9/-0) · still open — the silent-summary-loss product trade-off below needs a maintainer call · newly introduced — nothing blocking.

Verdict: no new blocking issues on this head; what remains is a product decision, not a code defect.

Re-verified the core mechanics on the new head: occasion_size_exceeded (backend/config/occasion_conversations.py) checks the 1,500-segment cap before the 786,432-byte estimate (256 KiB headroom under Firestore's 1 MiB), importing the estimator from the pure-leaf utils/firestore_document_size so config stays free of database._client; the size gate in backend/routers/listen/conversations.py is wired into _resume_continuation, reconnect resume, prepare, and lifecycle_loop, so an oversized row rolls over even outside occasion mode; _update_row_if_fits (backend/database/conversation_quiet_checks.py) skips arming stamps with 64 KiB headroom instead of aborting the transaction; and the enrichment consumer retires the obligation as DISCARDED/SUPERSEDED on size-limit errors. The duck-typed InvalidArgument match in backend/utils/stt/outcomes.py ("exceeds the maximum allowed size" over the exception chain) is slightly more fragile than the typed is_document_size_limit_error used in failure_reason.py, but it keeps this module free of a database._client import and is a reasonable trade-off — both paths are unit-tested.

Performance: the new checks are local byte estimates of an already-loaded row (no extra IO) at rollover decision points — negligible next to the stuck-retry loops they eliminate.
Security/data-handling: no new inputs reach Firestore paths beyond uid/conversation ids; the string match runs on SDK-raised error messages, not user data; no workflow/dependency/secrets changes.
Blast radius: users whose single conversations grow past ~768 KiB or 1,500 segments.

USER IMPACT: very long recordings now roll into a fresh conversation instead of hanging as an endless "processing" row; a conversation that still hits the ceiling completes with its transcript but without a generated summary.

What I'd still want: the maintainer product call on that silent summary loss — summary_delivery.refusal now returns document_size_limit (backend/utils/conversations/summary_delivery.py) and dead-letter terminals close oversized rows visible-but-summaryless, so confirm summaryless-complete is the intended UX — plus an optional backend changelog fragment for the behavior change.

---

by AI on behalf of David — if you need David’s attention urgently, please @Git-on-my-level and escalate with need human response.

[ocakciomursel-pixel]

@Git-on-my-level need human response

Thanks for confirming that the terminalization failure path is now covered and that no blocking code issue remains on head 75344d9. Could a human maintainer decide the remaining product behavior: whether an oversized conversation should remain visible with its transcript and complete without a generated summary when summary storage is permanently rejected? The alternative UX/notification requirement needs a product decision before adding more scope. Current-head checks report no failures, but this does not establish live production acceptance. No further source change is proposed solely to repeat the completed review.

[Git-on-my-level]

Thanks! Main has since landed part of this: oversized sync/enrichment holds (#23353), closing oversized in-progress rows on listen (#20663), and sync rollover near 1 MiB. Could you merge the latest main and trim the PR to what's still missing: (1) handling the size-limit error in the conversation_finalization worker, and (2) the live rollover at ~768 KiB / 1,500 segments, behind a kill switch (on unless set off), since it visibly splits long meetings? Please also avoid re-serializing the whole doc on every lifecycle tick (use an incremental estimate). Then we'll review and merge.

[ocakciomursel-pixel]

Merged the latest main (478e76ac15) and trimmed the branch in 8d4bf4f56e to only the remaining items:

1. conversation_finalization worker size-limit handling (backend/routers/conversation_finalization.py): Catches conversation_at_size_limit in run_listen_finalization_job, closes the oversized in_progress row via lifecycle.close_oversized_in_progress_conversation, and dead-letters the job with failure_code='document_size_limit'.
2. Kill-switched live rollover (backend/config/occasion_conversations.py, backend/routers/listen/conversations.py): Gates the ~768 KiB / 1,500-segment live rollover behind LIVE_CONVERSATION_SIZE_ROLLOVER_ENABLED (on by default unless set to 0/false/off/no, registered in config/feature-flags.yaml).
3. Incremental byte estimation (estimate_live_conversation_bytes_incremental): Caches non-segment document overhead and settled prefix segment bytes in LiveConversationController._size_estimate_state so each lifecycle tick only sizes newly settled segments plus the active tail segment instead of re-traversing the whole document.

[Git-on-my-level]

Since my last review: addressed - backend/utils/sync/pipeline.py is gone from the diff after the rebase; main's own sync commit (aedabea, 'fix(sync): attribute size rollover and rejected payloads', 2026-10-27) landed that side, so nothing was lost - still open - the maintainer call on default-on mid-meeting rollover (long captures splitting into several conversations at ~768 KiB / 1,500 segments) - newly introduced - a failing type gate and a registry date regression, both below.

Verdict: needs work before merge - two mechanical blockers; the overall design (terminalize the finalization dead-letter path + live rollover behind a kill switch) is sound and well-tested.

Blocking

1. CI: Backend unit shard 1/6 fails at the type gate with 9 pyright errors, all in your new code in backend/config/occasion_conversations.py:
   • Line 81: from utils.firestore_document_size import _value_bytes is reportPrivateUsage. Please expose a public per-value sizing helper in utils/firestore_document_size.py (or put the incremental estimator there) instead of importing the private one.
   • Lines 106/116/126 and 113/120/128/135/136: len(segments) / segments[...] on Sequence | None (reportArgumentType / reportOptionalSubscript) - pyright cannot narrow through the has_segments alias. Use an inline isinstance(segments, Sequence) and not isinstance(segments, (str, bytes, bytearray)) guard (or a TypeIs helper) before indexing.
2. backend/docs/feature-flag-registry.md: the as-of marker and the Overdue for a decision date move backward from 2026-10-22 to 2026-09-24 while main is at 2026-10-22 - the rows look like they were added to a stale local copy. The new LIVE_CONVERSATION_SIZE_ROLLOVER_ENABLED rows themselves match config/feature-flags.yaml; please rebase the file, set the as-of date forward, and re-check the overdue section against current main.

Non-blocking per-file notes

• backend/routers/conversation_finalization.py: _terminalize_oversized_finalization_job is correctly lease-fenced (409 completion_conflict on stale generation/epoch), honors an already-completed fanout, and dead-letters with failure_code='document_size_limit' from both exception paths. The chosen trade-off - if close_oversized_in_progress_conversation raises you still dead-letter (warning logged) and leave the conversation to stale-in-progress recovery - is reasonable; just be aware the prefix's processing then waits on that recovery path.
• backend/routers/listen/conversations.py: size checks wired at all three join/attach points (_resume_continuation, create_new_in_progress_conversation, prepare) and in lifecycle_loop; prepare() forcing finalize-with-content is the intended rollover semantics.
• backend/config/occasion_conversations.py: the incremental estimator is the right shape - per tick it only sizes newly settled segments plus the active tail, with marker-based invalidation that handles truncation/replacement, and non-segment overhead recomputed only when the top-level key set or photos length changes.
• config/feature-flags.yaml: new ops_kill entry registered fail:closed, consistent with the registry row.
• backend/tests/unit/test_listen_occasion_conversations.py: good coverage - both cap dimensions, all four kill-switch off values ('0'/'false'/'OFF'/'no'), re-enable rollover on the next tick, and incremental-vs-full estimator equivalence including the only-new-segments call check.
• backend/tests/unit/test_oversized_in_progress_close.py: covers both the dead-letter-on-close-raise and completed-fanout branches.

Performance: per-tick sizing is incremental - no new queries, no full-document re-traversal. Security: no new trust surfaces; Firestore paths are built server-side from uid/conversation id. Blast radius: long-capture users and the finalization job queue.

USER IMPACT: once this ships (kill switch default-on), very long recordings will appear as several consecutive conversations - each with its own summary - instead of one, rather than eventually failing to save near Firestore's 1 MiB limit.

Still want as a reviewer: green Backend unit suite on the head after the type fixes, the registry as-of date fixed forward, and a maintainer call on whether mid-meeting splitting at ~768 KiB / 1,500 segments should be default-on - the same product trade-off flagged on the earlier head. needs-maintainer-review stays up for that decision; the two blockers above are mechanical.

---

by AI on behalf of David — if you need David’s attention urgently, please @Git-on-my-level and escalate with need human response.

[ocakciomursel-pixel]

Addressed both blockers in 5b0d13e675 (merged latest main �8ff6c68a9):

1. Pyright fixes (�ackend/utils/firestore_document_size.py, �ackend/config/occasion_conversations.py): Exposed public estimate_firestore_value_bytes(value) instead of importing _value_bytes, and replaced the intermediate has_segments boolean with an inline isinstance(segments, Sequence) guard so pyright narrows segments cleanly (� errors).
2. Feature-flag registry date (�ackend/docs/feature-flag-registry.md): Re-rendered on top of current main (--as-of 2026-10-22) so the diff only adds the LIVE_CONVERSATION_SIZE_ROLLOVER_ENABLED rows without regressing �s-of or Overdue for a decision.