Repository navigation
fix(tests): refresh storefront catalog revision goldens - #24389
Conversation
Record revision 13 from the approved Omi Pro display-name change in the English, localized, and mirrored Flutter storefront fixtures. Preserve every other fixture byte, including all prices and entitlements.
Add the required internal-only mobile fragment for the mirrored fixtures.
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. |
There was a problem hiding this comment.
1 issue found across 53 files
Confidence score: 3/5
- The revision bump in
plus_budget80.jsonleaves the hardcoded revision assertion inplan_storefront_test.dartstale, so the Dart test will fail. Update the expected revision to match the fixtures.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="app/test/fixtures/storefront_v2/plus_budget80.json">
<violation number="1" location="app/test/fixtures/storefront_v2/plus_budget80.json:3">
P2: This revision bump breaks the Dart test that parses this fixture set. `app/test/models/plan_storefront_test.dart` still hardcodes `expect(data.catalogRevision, 12);` for `plus_monthly_yearly_toggle.json`, which now reports 13. Update that assertion to 13 in the same PR.</violation>
</file>
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
| { | ||
| "schema_version": 2, | ||
| "catalog_revision": 12, | ||
| "catalog_revision": 13, |
There was a problem hiding this comment.
P2: This revision bump breaks the Dart test that parses this fixture set. app/test/models/plan_storefront_test.dart still hardcodes expect(data.catalogRevision, 12); for plus_monthly_yearly_toggle.json, which now reports 13. Update that assertion to 13 in the same PR.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At app/test/fixtures/storefront_v2/plus_budget80.json, line 3:
<comment>This revision bump breaks the Dart test that parses this fixture set. `app/test/models/plan_storefront_test.dart` still hardcodes `expect(data.catalogRevision, 12);` for `plus_monthly_yearly_toggle.json`, which now reports 13. Update that assertion to 13 in the same PR.</comment>
<file context>
@@ -1,6 +1,6 @@
{
"schema_version": 2,
- "catalog_revision": 12,
+ "catalog_revision": 13,
"show_subscription_ui": true,
"header": "You've used 80% of this month's usage",
</file context>
Keep the Flutter fixture consumer pinned to catalog revision 13 alongside the regenerated golden. Preserve all existing price and entitlement checks.
|
Addressed the fixture-consumer revision mismatch in e534058: The final candidate is 3baaed9 after syncing current main. Its PR diff is still exactly 52 revision-only fixture updates, the matching one-line parser assertion and the internal-only changelog fragment. All price, entitlement, policy and copy bytes and assertions remain unchanged. Full final-head CI and the second delta review are the remaining merge gates. |
Record Pinecone provider query attempts and returned read units by the shared namespace family, actual route mode (`legacy` or `partitioned`), and a closed set of explicit callers. Unknown labels map to `other`; no UID or partition namespace is exported. Missing usage contributes no inferred units and has a separate counter. Metering errors are swallowed and counted. All 11 query sites in `backend/database/vector_db.py` pass their function name to `OwnerIndex.query`. Serving account-purge freshness verification is the one direct `_query` bypass and is separately metered as `account_purge_verify`, preserving headers, retries and routing. Operator migration queries, fetch/list and mutations are outside this query meter. No route/copy/mutation behavior changes; ships on without a flag. Live response-shape proof: pinned `pinecone==7.3.0`, `memories-backend-dev`, synthetic-owner-only query, no matches or writes, metadata/values disabled. Response type `QueryResponse`, usage type `pinecone.core.openapi.db_data.model.usage.Usage`, `usage.read_units == 1` (integer). The meter handles SDK attributes and mapping shapes. No production account data read. Integrated local validation on `648ab1813aa5afd12652f2b3083654394ee1d87a`: **424 tests passed** via `nice -n 19 bash backend/test.sh`, selected with `BACKEND_UNIT_TEST_FILE_LIST` and run through `omi-heavy` (26 metering, 53 owner-routing, 345 storefront tests). Coverage includes real SDK models, integer/zero/missing/invalid units, fraction/float/boolean rejection, bounded labels, failed provider calls, exporter failures, preserved arguments/results, both account-purge copies, sync/async caller wiring and sync scrape admission. The independent storefront baseline fix is included from main (#24389); the PR diff remains seven meter files. `make preflight` and `scripts/pr-preflight --metadata-only --pr-body-file <draft>` passed with the committed candidate and tracked files unchanged throughout. A local no-network smoke drove the actual `OwnerIndex.query` and `utils.metrics.metrics_response` with a pinned SDK response: the default-registry exporter contained one query and one read unit, and no owner label. Usage validation follows the pinned client's integer-only [data-plane schema](https://raw.githubusercontent.com/pinecone-io/pinecone-api/main/2025-04/db_data_2025-04.oas.yaml). GCP read tier: `ro-prod`; only infrastructure metric aggregates were read. Luna review and GitHub CI are separate merge gates. Read-only `ro-prod` GMP aggregates on 2026-11-02 confirm existing Cloud Run application scraping for backend, desktop-backend, sync, sync-backfill, integration and notifications. The sync sidecar allowlist now admits `omi_pinecone_.*`. GKE listen/pusher use authenticated local Prometheus `/metrics` jobs but were absent from GMP; the GPU batch worker uses a separate registry and performs no Pinecone queries. The local Prometheus sync bridge does not import these counters. Details and acceptance commands: `backend/docs/runbooks/pinecone-query-metering.md`. Dashboard-free query: **which caller spends the most reported read units over 24h**, for the GMP Cloud Run scope: ```promql topk(10, sum by (caller) ( increase(omi_pinecone_read_units_total{project_id="based-hardware",cluster="__run__"}[24h]) )) ``` Inspect `omi_pinecone_query_missing_usage_total` and `omi_pinecone_query_metering_errors_total` alongside it. GMP totals do not include listen/pusher until that collection gap is resolved or local series are analyzed separately. This meter does not establish a root cause or savings. Deployment: rollout lead to include backend image plus revision-local sync scrape profile in the next backend train; listen/pusher need their own image rolls to emit this instrumentation. Verify running release identity and newly emitted samples after deploy. No deployment performed by this lane. ## Product invariants affected none
What changed and why
Refresh storefront snapshots from catalog revision 12 to 13 after #24367's approved Unlimited → Omi Pro display-name change. Update the Flutter fixture consumer's matching revision assertion so the mirrored fixtures keep parsing successfully.
All 52 fixture payloads differ only in
catalog_revision; every price, entitlement, policy and copy byte is unchanged. Add the requiredkind: nonemobile changelog fragment for this internal test correction.Product invariants affected
none
How it was verified
catalog_revision: 12 → 13, permitting only the matching Dart assertion and internal-only fragment alongside it.BACKEND_UNIT_TEST_FILE_LIST=<storefront-list> BACKEND_PYTEST_WORKERS=4 bash backend/test.sh: 345 passed.flutter test --concurrency=1 test/models/plan_storefront_test.dart: 19 passed.backend/test.shpassed all 38 tests. No cap-hold code, assertions or timing budgets were changed.Local tests ran serially through
omi-heavyandnice -n 19. Normal push hooks remain enabled.Tests
Existing regression checks:
test_golden_storefront_scenarios,test_localized_storefront_goldens, and the Flutter storefront model suite. Complete payload, pricing, entitlement and parser rejection assertions remain intact.UI evidence
UI-Evidence: none -- fixture metadata and a matching test assertion; no runtime UI change.
Failure class (fixes)
Failure-Class: none