Repository navigation
test(billing): preserve entitlement golden across Omi Pro display rename - #24378
Merged
Merged
Conversation
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. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
This was referenced Nov 2, 2026
Git-on-my-level
added a commit
that referenced
this pull request
Nov 2, 2026
) ## Why David, 2026-11-02 06:09Z: "We should be processing most audio within 30 mins of finalization, and alarm on 2 hours." Failed-held, quarantined and repair waits must count, because that is how 165 segments were lost on 11-01. The existing projection gauge reads 14.9 days of held backlog, so any plain age alarm would fire forever on day one. ## What The batch queue sample (`QueueMetrics.refresh`, FirestoreStore only) exports, at most once per 5 minutes per process: | series | meaning | |---|---| | `omi_batch_timeliness_new_oldest_age_seconds{leg}` | oldest unpublished unit admitted at or after the epoch, all clocked from admission: `uncommitted` (policy and vendor holds skipped), `quarantined` (every recovery state), `assignment_held` (committed, projection receipt quarantined), `projection`; `quarantine_held_rows` visible only | | `omi_batch_timeliness_new_over_sla_units{leg}` | new-cohort units past 2 h; also `terminal_empty` (visible only) | | `omi_batch_timeliness_backlog_units{leg}` | units from before the epoch: visible counts, never alarmed | | `omi_batch_timeliness_scan_truncated{leg}` / `omi_batch_timeliness_samples_total{outcome}` | blind-spot flag and sample counter | - **Kill switch:** `BATCH_TIMELINESS_ALARM_ENABLED`, default on, registered as `ops_kill`. - **Epoch:** `BATCH_TIMELINESS_EPOCH`, default `2026-11-02T06:09:00Z`, when the target was set. - **Cadence:** `BATCH_TIMELINESS_INTERVAL_SECONDS`, default 300. - **Terminal-empty switch:** `TERMINAL_EMPTY_IS_PROCESSED` in `config/batch_timeliness.py`, **default `True`** (see below). - **Alert-only:** no scheduler, ramp, gate or hard-3 rollback trigger reads these series. - **Cost:** under 300 reads per sample per process, at most about $0.4–0.8/day across 16 workers. - **Indexes:** two new jobs composites, declared in `database/batch_pipeline_indexes.py` with `firestore.indexes.json` regenerated: `(state, recovery_state, enqueued_at)` for `assignment_held`, and `(state, recovery_state, recovery.failure.reason, measured_held_vad_speech_seconds, enqueued_at)` for `terminal_empty`. Until both are READY the sample counts `error` and the stale rule fires. - **Rules:** three agent-tier Grafana rules sit in `charts/monitoring/pending/batch-timeliness.json`, because the monitoring contracts reject rules on series that don't exist in prod yet: - `omi-batch-unit-unpublished-2h`: max new age over the uncommitted, quarantined, assignment_held and projection legs > 7200 s, with a fresh sample. - `omi-batch-timeliness-sample-stale` - `omi-batch-timeliness-scan-blind`: any leg's walk full for 30 minutes. - **Apply step:** in `charts/monitoring/batch-projection/README.md`. ## Terminal empty_asr counts as processed (David, 2026-11-02 10:45Z) David: "finished recording counts as processed". The lead ruled the one-line switch `TERMINAL_EMPTY_IS_PROCESSED` defaults that way, alarm only, with no speech threshold. - **Default `True`:** units that are `failed_held` with recovery failure `empty_asr` and measured held VAD speech exactly 0 leave the `quarantined` leg. Its count subtracts them exactly, so a page of them can hide a younger unit but never a breach. - **`False`:** puts them back in the 2h alarm. - Units with measured speech always stay in the alarm. `terminal_empty` stays a visible count either way. - "No recoverable speech" reuses the existing silence contract (`held_silence_allowed`: census exactly 0). Measured (ro-prod, 10:36Z, counts only): no quarantined `empty_asr` failed-held unit had zero measured speech. All 13 admitted since the epoch carry some: 10 under 2 s, 2 at 2–10 s, 1 over 10 s. Of 530 backlog units, 137 have no VAD evidence at all. So the default changes nothing in today's alarm. ## Measured (ro-prod, read-only, 2026-11-02) - Day-one values at the 06:09Z epoch: new breaches 0. Backlog: quarantined 609, sibling_held 16,921, uncommitted including holds 523, projection 7. - Fire test, the PR's real code against prod (`fire_test.py`): epoch 03:00Z FIRES ($A = 18,155 s); epoch 06:09Z is clear. - Failed-held arrives at about 130/day (115 `empty_asr`), so once deployed the alarm keeps firing until those are resolved within 2 h. That is the intended signal. ## Round 1 fixes (Luna FIX-FIRST, lead ruling 2cef45) - The projection leg clocks from the job's `enqueued_at` (oldest 50 pending results, then job reads), not from commit. - Live vendor holds are walked apart from active work. Truncation is a per-leg gauge. - New `assignment_held` leg, the real holes per siblingrelease. - Read ceiling corrected: under 300 reads per sample. - The sample comes from `store.client` instead of a new store method, which fixes `test_jobstore_protocol_parity`. ## Round 2 delta (lead accepted 5e44c47 without a round 3, 11:42Z) - Terminal-empty switch and `terminal_empty` count, above. - A walk ordered by a clock other than admission (ready by `available_at`, retry by `retry_started_at`) reports truncation on a full page even when it found admitted rows, since an older admission can sit behind it. The blind rule now reads every leg. - Switch default `True` per David's ruling. - Merged origin/main, including the main-red fix #24378 (shard 4/6 goldens). ## Tests - Red first: the new tests fail without the switch (ImportError of `TERMINAL_EMPTY_IS_PROCESSED`). - Green through `backend/test.sh`: `test_batch_pipeline_timeliness.py` (15), `test_batch_pipeline_metrics.py`, `test_entitlement_versions.py`, `test_storefront_presentation.py`, and the Firestore index/query-shape guard (`test_firestore_query_shapes`, `test_firestore_indexes`, `test_firestore_index_config`, `test_firestore_index_selector_contract`, `test_firestore_query_contract`). ## Deploy Batch worker image (lead's train), after the index reconcile. Rollback: set `BATCH_TIMELINESS_ALARM_ENABLED=false`, or revert. Nothing else reads the series. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Backend shard 4 rejects #24367's approved Unlimited → Omi Pro display rename because the legacy entitlement snapshot includes
definition.display_name. The complete catalog diff in that PR is revision 12 → 13 and theunlimited_v2display name; its runtime change is the matching plan-card title. No price identity, allocation, feature, access rule, or billing behavior changed.Keep the original historical golden untouched and apply only the exact
unlimited_v2/ Unlimited → Omi Pro rename to the expected snapshot. The byte comparison still pins all fields, including the new display name, and each price's runtime resolver policy is compared against the same adjusted expectation. Arbitrary display-name changes still fail.Validation:
omi-heavy run -- nice -n 19 env BACKEND_UNIT_TEST_FILE_LIST=<entitlement-tests.txt> BACKEND_PYTEST_WORKERS=1 bash backend/test.shpassed all 58 entitlement-version tests. A separate subscription-restructure shard withBACKEND_PYTEST_MARK_EXPR='not integration'passed all 36 tests, including the Omi Pro catalog title assertions. The complete standard pre-push gate passed, including the shared repository preflight, catalog compiler contract, Pyright, and the 58-test changed shard again. Luna round 1 returned SHIP with no findings. Exact head2580a5913ebf0068a2694f0fda2c09f9197b28e9reached terminal CI: 39 success, 25 skipped, 1 neutral in the full check rollup; no failures, pending or cancelled checks. No reruns were needed. This changes tests only; no endpoint or deployment is needed.Failure-Class: none
This repairs a test expectation for an approved display-name change; it changes no product behavior or entitlement contract and preserves the complete legacy runtime snapshot guard.