[aiwithrajan]

Description

Validates non-negative prices, bounds, and typing in storefront models.
Currently:

1. StorefrontPrice defines amount_minor: int without lower-bound constraints, allowing negative price amounts to be deserialized.
2. months_free in StorefrontPrice and catalog_revision in StorefrontResponse lack non-negative bounds (ge=0).
3. percent in StorefrontUsage lacks range bounds (ge=0, le=100).
4. Uses standard Optional[...] across storefront models for typing consistency.

This change:

• Adds ge=0 bounds to amount_minor, months_free, and catalog_revision.
• Constrains percent in StorefrontUsage to [0, 100].
• Enforces non-empty currency strings (min_length=1, max_length=10).
• Adds hermetic unit test coverage in backend/tests/unit/test_storefront_validation.py.

Test Plan

• Run unit tests:

  PYTHONPATH=backend python3 backend/tests/unit/test_storefront_validation.py

  Result: 9/9 tests passed in 0.005s.

[Git-on-my-level]

Thanks @aiwithrajan — the non-negative bounds you added are largely right, and I appreciate the hermetic tests. One blocking issue before this can merge.

Blocking: the 100 cap on usage percent breaks the exhausted-budget surface

• backend/models/storefront.py: percent: Optional[int] = Field(default=None, ge=0, le=100) — but percent at and above 100 is a documented state, not invalid data. backend/docs/storefront-contract.md defines the exhausted surface as usage_budget with "usage percent at least 100" (the same split that selects the You've used all of this month's usage header).
• Production source: backend/routers/storefront.py _usage accepts budget_percent from the usage projection guarded only by percent >= 0 and finiteness — there is no upper clamp anywhere, and backend/utils/storefront_copy.py present_usage/_wire_percent round but never clamp.
• Effect on this head: backend/tests/unit/test_storefront_presentation.py parametrizes the cap surface with percent=140 (test_pro_budget_cap_recommends_architect_checkout, test_legacy_price_pro_at_cap_lists_architect_without_checkout, test_pro_at_cap_without_purchasable_architect_price_is_not_recommended) via StorefrontUsage(**present_usage(...)), which now raises ValidationError. In production, the broad except in _usage would catch that and degrade an over-budget user to kind: "unavailable" — showing the 80% header instead of the exhausted-budget surface and hiding the Architect upgrade card exactly for the users who need it.

Requested change: remove le=100 and keep ge=0. If you believe the wire should cap at 100, that's a product/contract decision — it needs a clamp at the presentation layer plus contract and test updates, so please raise it with a maintainer rather than encoding it in the model.

What looks good

• amount_minor: ge=0: matches every producer (paid rows are filtered to unit_amount > 0 in _valid_price_rows; the Free tier is intentionally 0 — your zero-amount test covers that correctly).
• months_free ge=0 and catalog_revision ge=0: producers are months_free() (>=1 or None) and CATALOG_REVISION (currently 7) — safe.
• currency min_length=1/max_length=10: fine for ISO codes; producers are Stripe's usd or the ladder config.
• backend/tests/unit/test_storefront_validation.py: hermetic, and the from models.storefront import ... convention matches the existing suite. One note on the test plan: running only this file shows 9/9 green, but the checks on this PR only include the AI reviewer — the backend unit suite hasn't run. It would have caught the percent=140 failures above; please run the full backend unit suite (or push so CI picks it up) after the change.

Minor, non-blocking: the str | None to Optional[str] swap is behaviorally identical but reverses the file's (and the surrounding backend's) PEP 604 style; consider dropping that churn so the diff is only the validation change.

The non-negative bounds are a genuine hardening win — with the le=100 cap removed this is in good shape.

---

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

[aiwithrajan]

Updated: removed the le=100 constraint on StorefrontUsage.percent to preserve over-budget presentation states (e.g. percent=140), updated unit tests, and added Failure-Class: validation-gap trailer. Thank you!

Dismissed: the blocking concern (le=100 cap on StorefrontUsage.percent breaking the documented exhausted-budget surface, where usage percent at least 100 is valid per backend/docs/storefront-contract.md) is resolved at 183a54d — percent is now Field(default=None, ge=0) with no upper cap, and the boundary test asserts percent 0/100/140 all validate.

[Git-on-my-level]

Thanks @aiwithrajan — this follow-up resolves the blocking issue from my earlier review.

• backend/models/storefront.py: percent: Optional[int] = Field(default=None, ge=0) — the le=100 cap is gone. That matches the contract (backend/docs/storefront-contract.md defines the exhausted surface as usage_budget with usage percent at least 100) and the producer guard in backend/routers/storefront.py _usage, which accepts budget_percent when finite and >= 0 with no upper clamp. Over-budget users keep the exhausted-budget surface and the Architect upgrade card.
• backend/tests/unit/test_storefront_validation.py: the boundary test now asserts percent 0, 100, and 140 all validate — exactly the states the old cap would have rejected. The negative-percent, negative-amount, negative-months-free, and negative-catalog-revision rejections, plus the intentional zero-amount Free tier case, are all good coverage.

One remaining gate: no checks have run on this head yet (183a54d). Please make sure the backend unit suite runs green here — in particular backend/tests/unit/test_storefront_presentation.py, which parametrizes percent=140 through StorefrontUsage(**present_usage(...)) and would have caught the earlier cap.

The str | None → Optional[...] swap is still style churn against the file's PEP 604 convention, but it's behaviorally identical and non-blocking.

With CI green this is in good shape for a maintainer to pick up.

---

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

[github-actions]

Hey @aiwithrajan 👋

Thank you so much for taking the time to contribute to Omi! We truly appreciate you putting in the effort to submit this pull request.

After careful review, we've decided not to merge this particular PR. Please don't take this personally — we genuinely try to merge as many contributions as possible, but sometimes we have to make tough calls based on:

• Project standards — Ensuring consistency across the codebase
• User needs — Making sure changes align with what our users need
• Code best practices — Maintaining code quality and maintainability
• Project direction — Keeping aligned with our product principles and locked invariants

Before your next PR, please skim:

• PRODUCT.md — product north star
• Product invariants — locked rules (shared chat, memory tiers, agent control plane, integrations, brand)

If this was declined for direction or taste, maintainers should cite an invariant ID or open a proposed one — ask if that citation is missing.

Your contribution is still valuable to us, and we'd love to see you contribute again in the future! If you'd like feedback on how to improve this PR or want to discuss alternative approaches, please don't hesitate to reach out.

Thank you for being part of the Omi community!