Skip to content

Show model tiers only where a deployment bills by them - #381

Open
LiminalDevving wants to merge 1 commit into
mainfrom
oss/model-tier-facts
Open

LiminalDevving wants to merge 1 commit into
mainfrom
oss/model-tier-facts

Conversation

@LiminalDevving

@LiminalDevving LiminalDevving commented Oct 24, 2026 •

Copy link
Copy Markdown
Contributor

Show model tiers only where a deployment bills by them

A model's subscription tier is a billing grade. Only a linked meter uses it; the NoopMeter the open-source build falls back to ignores it. The model picker still showed tiers everywhere: a Tier tab, and a tier badge on every model. Because the seed sets no tier, every model read T3, with a tooltip about plans the install doesn't have.

What changes

  • GET /api/model sends subscription_tier only when a meter is linked. metering.Linked() reports metering.New != nil; the model handler clears the tier otherwise, and the field is omitempty. In openapi.yaml the field is no longer required, and its description says when it is present.
  • The picker shows its Tier tab only when a listed model carries a tier. Tier badges already hid for a model without one.
  • A build that links a meter keeps its tiers, with nothing new to bind or replace.
  • Comments on the tier values described one deployment's plans. They now say what a tier is: a billing grade a linked meter may use, with no effect otherwise. The frontend model's comment listed the wrong values.

Screenshots

Before: a Tier tab, and T3 on every model.
before-picker

After: no Tier tab and no badges.
after-picker

Testing

  • make pre-commit and check-package-summaries
  • New internal/handlers/model/handler_test.go: the tier is absent without a linked meter and sent with one
  • Model picker specs: the Tier tab and badges appear when models carry tiers, and neither appears when none do
  • Frontend unit suite (215 files, 2,215 tests), npm run build:prod, npm run lint:e2e
  • Against an API built from this branch, GET /api/model returned 41 models, none with a tier
  • Visual suite in the CI container: no baselines change
  • Mock e2e on Chromium desktop and mobile: 258 passed. The failures are ones main already has: the three mobile thread-import specs, and two mobile specs that open the profile straight after a reload, which Add provider keys in the app #380 fixes.

A model's subscription tier matters only to a linked meter; the meter
the open-source build falls back to ignores it. The app still showed a
Tier tab in the model picker and a tier badge on every model, all
reading T3 because the seed sets no tier, with a tooltip about plans
that don't exist.

GET /api/model now sends subscription_tier only when a meter is linked
(metering.Linked), and the picker shows its Tier tab only when a listed
model carries a tier. Badges already hid without one. A build that
links a meter keeps its tiers, with nothing to bind.

Comments on the tier values described one deployment's plans; they now
say what a tier is. The frontend model's comment listed the wrong
values.
@blue-door-code-compass

Copy link
Copy Markdown

Chat-App API Regression

  • Commit: 5db394b
  • Status: SKIPPED
  • Detail: regression orchestrator script not present in branch

Regression suite was not found in this branch, so this check was skipped.

@blue-door-code-compass

Copy link
Copy Markdown

Reviewed commit: 5db394b
✅ Code review PASSED (gating: major)
✅ Approved by code-bot

🔧 Minor

internal/handlers/model/handler.go:61-66
Best Practices / Documentation
In ListModels, tiers are cleared by setting SubscriptionTier to the empty string and relying on omitempty to drop the field. This is fine given the Go struct tag, but it subtly couples handler logic to JSON tags. Consider adding a short comment next to the loop (or on the field tag) clarifying that an empty string deliberately causes omission in the API response, to avoid future refactors breaking this behavior unintentionally.

internal/handlers/model/handler.go:61-67
Maintainability / Complexity
The ListModels handler directly mutates the modelList objects returned from ListModels before encoding. If the slice or its elements are reused elsewhere in the future (e.g., caching, logging, or additional response construction), this could lead to surprising side effects. Consider either: (a) cloning modelList into a response-specific slice before mutation, or (b) clearly documenting that ListModels may mutate SubscriptionTier on its results so other callers do not assume immutability.

🧹 Nitpick

internal/handlers/model/handler_test.go:48-60
Best Practices / Other
The new tiersApply function field is only configurable via direct struct construction in tests. To keep the constructor as the single entry point for production and ease future extension, consider adding an unexported constructor (or a test-only helper) that accepts a tiersApply override, rather than constructing Handler manually in tests.

internal/handlers/model/handler_test.go:9-70
Best Practices / Other
The test helper listModels uses a real SQLite database plus Ent just to validate presence/absence of subscription_tier. This gives good integration coverage but is relatively heavy and may slow the test suite as it grows. If test performance ever becomes an issue, consider factoring the tier-stripping logic into a small pure function (e.g. stripTiersIfNotLinked(models []models.Model, tiersApply func()bool)) with focused unit tests, while keeping one integration test to cover wiring.

@blue-door-code-compass blue-door-code-compass 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.

✅ Code review complete - Approved by code-bot

@codecov-commenter

codecov-commenter commented Oct 24, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 87.50000% with 2 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
.../components/model-picker/model-picker.component.ts 75.00% 2 Missing ⚠️

📢 Thoughts on this report? Let us know!

@github-actions

Copy link
Copy Markdown

E2E · mock LLM ✅

166 passed, 7 skipped · 501s · 5db394b

Suite This PR main
chrome ✅ 136/136 136/136
mobile – add the e2e-full label 236/253 ❌
visual ✅ 30/30 30/30

HTML report · run · main baseline: 83704ad

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.

3 participants