Repository navigation
feat(settings): preview saved model suitability and estimated cost - #8461
Conversation
…cached-token cost
…cached-token cost
|
Warning Review limit reached
This review includes 5 billable files and costs up to $1.25. Or wait 49 minutes for your next included review. View limit details
Comment |
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. |
Tiny Sweeper review
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 692b65fbc5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| export function assessModelFeasibility<T extends FeasibilityModel>( | ||
| models: readonly T[], | ||
| requirements: ModelRequirements | ||
| ): ModelFeasibilityReport<T> { |
There was a problem hiding this comment.
In the desktop Settings routing view, assessModelFeasibility now decides hard exclusions, unknown-price semantics, and rank order entirely in TypeScript. Those are model/cost business rules, so other hosts cannot reuse them and the result can already disagree with core accounting: crates/openhuman-core/src/agent/cost.rs::is_free_model recognizes :free as a known zero-cost route, while the new positive-price checks classify every consumed zero price as unknown. Expose this assessment from a core operation/RPC and keep React responsible only for presentation.
AGENTS.md reference: AGENTS.md:L53-L56
Useful? React with 👍 / 👎.
| const reasonLabels: Record<ModelFitReason, [string, string]> = { | ||
| context_too_small: [ | ||
| 'settings.ai.feasibility.contextTooSmall', | ||
| 'Context smaller than input + output tokens', |
There was a problem hiding this comment.
For any non-English locale, a repo-wide search of app/src/lib/i18n finds none of the new settings.ai.feasibility.* keys, so useT() resolves every new label and reason to these English fallbacks; positive costs also remain the literal English Est. $ emitted by formatEstimatedModelCost. Add real entries for every locale and localize the formatted output, otherwise switching locale leaves the whole new section in English.
AGENTS.md reference: AGENTS.md:L318-L319
Useful? React with 👍 / 👎.
| <div className="grid grid-cols-2 gap-2 md:grid-cols-3 lg:grid-cols-5"> | ||
| <label className="space-y-1 text-xs text-content-secondary"> | ||
| <span>{t('settings.ai.feasibility.input', 'Estimated input tokens')}</span> | ||
| <input |
There was a problem hiding this comment.
In the Settings routing panel this starts a set of raw <input> controls, followed by a raw checkbox, <select>, <button>, and hand-rolled bordered surfaces, despite existing TextField, Checkbox, NativeSelect, Button, Card, and accordion/collapsible primitives. This bypasses shared focus, sizing, theme, and accessibility behavior, so fixes to those primitives will not reach the new panel; rebuild it from the shared UI layer.
AGENTS.md reference: AGENTS.md:L327-L331
Useful? React with 👍 / 👎.
| {t('settings.ai.feasibility.title', 'Model suitability and cost preview')} | ||
| </summary> | ||
| <div className="p-3 pt-0"> | ||
| <ModelFeasibilityPreview models={draft.modelRegistry} /> |
There was a problem hiding this comment.
about_app is the source used by capability search and the About catalog, but the inspected settings.configure_ai entry in crates/openhuman-core/src/platform/about_app/catalog_localai_settings_mobile.rs:69-78 still mentions only providers and routing and gives no way to discover this model preview. Update that capability's description/how-to text or add a dedicated capability when adding this surface, otherwise the product capability catalog remains stale.
AGENTS.md reference: AGENTS.md:L385-L385
Useful? React with 👍 / 👎.
| /> | ||
| ))} | ||
| </WorkloadTable> | ||
| {!embedded && ( |
There was a problem hiding this comment.
The new Settings → LLM → Routing disclosure is only exercised by isolated Vitest renders; a repo-wide search of app/test/e2e/specs found no feasibility or suitability flow. Add a mocked browser/desktop test that navigates to this entry point, expands it, edits constraints, and confirms the embedded onboarding surface excludes it, because mounting the component alone does not verify the real route and tab integration.
AGENTS.md reference: AGENTS.md:L186-L187
Useful? React with 👍 / 👎.
| export function formatEstimatedModelCost(cost: number | null): string { | ||
| if (cost === null || !Number.isFinite(cost) || cost < 0) return 'Cost unknown'; | ||
| if (cost > 0 && cost < 0.0001) return '<$0.0001'; | ||
| return 'Est. $' + cost.toFixed(4); |
There was a problem hiding this comment.
When an estimate is only slightly above a user-entered budget—for example, 1,004 input tokens at $10/1M produces $0.01004 against a $0.01 limit—the row is marked excluded, but this formatter renders Est. $0.0100, making the displayed amount appear equal to the limit. Preserve enough precision or round upward so the shown cost explains why the model was excluded.
Useful? React with 👍 / 👎.
Summary
Problem
When comparing configured models, the UI has no local way to evaluate context, vision and approximate cost requirements together. Users may mistakenly treat missing/zero price as verified free, or confuse a catalog entry with a live model authorization. Moving the fork's entire fleet/settings implementation upstream would mix this useful UX improvement with incompatible credential and provider contracts.
Solution
The new pure
modelFeasibility.tsmodule ranks only saved metadata and labels every assumption;ModelFeasibilityPreview.tsxrenders it in a collapsed, advisory section. The section appears only in Settings, not the embedded onboarding settings. The calculator uses no remote API. Hard constraint exclusions precede ranking by estimated saved-price cost. Cached input is priced separately and must not exceed input volume.Submission Checklist
modelFeasibility.test.tsandModelFeasibilityPreview.test.tsx.Impact
Frontend only (web/desktop). No credential writes, provider switches, routing updates, changes to the inference backend, or network egress. The disclosure control is collapsed by default to avoid cluttering existing routing UX. Display is based on saved metadata and does not verify runtime availability or current vendor prices.
Related
AI Authored PR Metadata (required for Codex/Linear PRs)
Linear Issue
Commit & Branch
AmirrezaFarnamTaheri:upstream/feat-saved-model-suitability-preview-20261027692b65fbc51a7fc89181cea072ac224aa094415dValidation Run
692b65fbc51a7fc89181cea072ac224aa094415d.pnpm --filter openhuman-app exec vitest run src/components/settings/panels/ai/modelFeasibility.test.ts src/components/settings/panels/ai/ModelFeasibilityPreview.test.tsxcovered by full hosted Vitest suite.Validation Blocked
Behavior Changes
Parity Contract
Duplicate / Superseded PR Handling
Review readiness (2026-10-28)