🤖 feat: validated make update-models refresh + full catalog in Treat as - #3930
Conversation
Upstream dropped max_pdf_size_mb entirely, so the capability inference test now exercises the extraction branch with injected metadata.
|
@codex review |
This comment has been minimized.
This comment has been minimized.
![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: e84d314ea1
ℹ️ 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".
- validate curated coverage by usable metadata, not key presence - bound catalog shrink against the vendored baseline - sort serialized keys to avoid reorder-only churn - normalize gateway catalog ids so rows match resolved metadata - order UPDATE_MODELS=1 refresh before catalog-consuming bundles - pin scheduled workflow checkout to main and route it via make
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dda73bb7b9
ℹ️ 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".
…fallback - capabilities now resolve with getModelStats' shared key preference so provider-scoped entries win in both lookups - models.json is a prerequisite of the main bundle so catalog-only refreshes rebuild it - update-models workflow references only canonical XUM_APP_* secrets
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: aa5110ee78
ℹ️ 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".
- catalog rows must resolve stats from their own entry (resolveRawModelEntry), excluding provider-scoped duplicates shadowed by models-extra overrides - validation compares priced chat-model coverage against the vendored baseline so an upstream pricing-field rename cannot zero-price the catalog
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4e04f5cd7f
ℹ️ 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".
- shrinkage measured over usable chat/responses entries (getModelStats bar), so token-limit loss and responses-mode loss both register - input- and output-priced chat coverage compared independently so renaming one cost field cannot hide behind the other
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 07dd882b64
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 673c0d8c8d
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Security Review
Here are some automated security review suggestions for this pull request.
Reviewed commit: 673c0d8c8d
Only the user who started this review can view the report in Codex.
ℹ️ About Codex security reviews in GitHub
This is an experimental Codex feature. Security reviews are triggered when:
- You comment "@codex security review"
- A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review
Once complete, Codex will leave suggestions, or a comment if no findings are found.
- summarizeCatalog tracks usable/input-priced/output-priced coverage per mode plus total entries; every baseline mode is compared independently, closing pricing/mode-slice regressions the aggregate counters hid - baselines below a noise floor (20) skip relative checks so tiny modes cannot block refreshes forever - curated coverage reuses generateModelLookupKeys instead of the xai/moonshot special case
Median input/output cost across mappable entries must stay within 100x of the vendored baseline, so a poisoned upstream cannot keep every field while scaling rates toward zero.
|
Codex Review: Didn't find any major issues. Swish! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 70578c3110
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Security Review
Here are some automated security review suggestions for this pull request.
Reviewed commit: 70578c3110
Only the user who started this review can view the report in Codex.
ℹ️ About Codex security reviews in GitHub
This is an experimental Codex feature. Security reviews are triggered when:
- You comment "@codex security review"
- A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review
Once complete, Codex will leave suggestions, or a comment if no findings are found.
…ic magnitude baselines Replaces the bespoke inputPriced/outputPriced counters and cost-only medians with generic guards over the whole retained-field surface: - per-mode coverage counts for every retained field (pricing incl. image/cache/ tiered rates, token limits, capability flags; booleans count only when true) - positive-sample counts and medians for every numeric field across mappable modes, rejecting >100x median shifts including collapses to zero - per-mode entry counts so whole-mode omissions are caught directly
Catalog-wide counts and medians cannot see a targeted repricing of a single surviving row. validateModelData now takes the baseline catalog itself (simplifying callers) and rejects any surviving entry whose positive numeric field shifts more than 100x or vanishes.
|
@codex review Addressed all 5 round-7 findings (4 review + 1 security) by generalizing the guards instead of adding more bespoke counters:
Please review the current head 67ccf59. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 67ccf596a1
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Security Review
Here are some automated security review suggestions for this pull request.
Reviewed commit: 67ccf596a1
Only the user who started this review can view the report in Codex.
ℹ️ About Codex security reviews in GitHub
This is an experimental Codex feature. Security reviews are triggered when:
- You comment "@codex security review"
- A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review
Once complete, Codex will leave suggestions, or a comment if no findings are found.
…tion commits Raw-key per-entry comparison missed alias shadowing: a poisoned catalog can add or substitute a higher-precedence lookup key that hijacks runtime resolution while all counts and medians stay intact. Validation now compares, for every model id either catalog can serve, the entry each side resolves through getModelStats' own lookup preference, using runtime parseNum semantics (numeric strings compare by value, not as collapses) and requiring baseline-true capability flags to stay true per identity. Also adds the sanctioned automation attribution footer to the weekly bot commit/PR and documents the convention in the pull-requests skill.
|
@codex review Addressed all 5 round-8 findings in c95ee88 by moving per-entry comparison from raw keys to runtime identities: for every model id either catalog can serve, both sides resolve their entry through getModelStats' own lookup preference and the resolved entries are compared. This covers alias-shadow additions and removed-key+alias substitutions with one mechanism, adds per-identity baseline-true capability-flag checks, and uses runtime parseNum semantics so numeric-string representation changes compare by value. The weekly bot commit/PR now carries the automation attribution footer, with the convention documented in the pull-requests skill. Verified against live upstream: Please review the current head c95ee88. |
There was a problem hiding this comment.
💡 Codex Security Review
Here are some automated security review suggestions for this pull request.
Reviewed commit: c95ee88162
Only the user who started this review can view the report in Codex.
ℹ️ About Codex security reviews in GitHub
This is an experimental Codex feature. Security reviews are triggered when:
- You comment "@codex security review"
- A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review
Once complete, Codex will leave suggestions, or a comment if no findings are found.
|
@codex review Re-requesting: the security review for head c95ee88 arrived, but the code review verdict for this head never landed. No code changes since the last request. The round-9 security finding (mode-flip repricing) was resolved with an explicit maintainer scope decision: the human-reviewed weekly bot PR is the trust boundary against a poisoned upstream; in-repo validation remains a best-effort sanity gate. Please review the current head c95ee88. |
|
Codex Review: Didn't find any major issues. Keep it up! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
This comment has been minimized.
This comment has been minimized.
1 participant
Summary
Adds a validated, idempotent
make update-modelsflow that refreshes the vendored LiteLLM model catalog, and expands the custom-model "Treat as" dropdown to offer the full models.json catalog instead of only the ~21 curatedKNOWN_MODELS.Fixes #3727
Background
src/common/utils/tokens/models.jsonis a pruned vendor copy of LiteLLM'smodel_prices_and_context_window.json, butscripts/update_models.tswas manual-only and unwired, so the data had gone stale (last real refresh around #1103). Meanwhile the "Treat as" mapping (mappedToModel) is persisted as a plain string and resolved throughresolveModelForMetadata->getModelStats, which can already resolve any catalog entry, so restricting the dropdown to curated models was purely a UI limitation. Per the maintainer suggestion on the issue, the refresh is a make-process flag that fetches, validates pricing, and updates only when needed.Implementation
Freshness:
scripts/update_models.tsis now a thin CLI over pure logic insrc/common/utils/tokens/updateModelsData.ts(covered bybun test src): prune to retained fields, drop entries whose present cost fields are not finite non-negative numbers, then validate before writing (>= 500 chat entries as a truncation guard, <= 5% invalid-pricing drops, and everyKNOWN_MODELSentry still resolvable via models.json plus models-extra). Validation also bounds shrinkage to 10% below the vendored catalog's chat-model count, curated coverage requires usable token limits (same bar as getModelStats) rather than key presence, and serialization sorts keys so upstream reordering never diffs. It writes only when the serialized content changed, so reruns are no-ops.make update-modelsruns it;make build UPDATE_MODELS=1makes the refresh a prerequisite of every catalog-consuming bundle, ordering it under parallel make and forcing those bundles stale (plainmake buildstays network-free)..github/workflows/update-models.ymlruns weekly (plus dispatch) and opens/updates abot/update-modelsPR only when the data changed, reusing the auto-cleanup GitHub App token pattern so the bot PR triggers CI; the checkout is pinned to main and the job goes throughmake update-models.knownModels.test.tsnow reuses the samefindMissingKnownModelscoverage check instead of duplicating the key logic.max_pdf_size_mbwholesale, so the capability test for that field now exercises the exported extraction function with injected metadata instead of a retired Gemini 1.5 fixture.Treat as:
listModelCatalogIds()(src/common/utils/tokens/modelCatalog.ts) returns canonicalprovider:modelids for every chat/responses catalog entry that resolves throughgetModelStats, unioned with the curated ids (~2300 ids on the refreshed data), normalized via normalizeToCanonical and deduped so gateway keys with direct-provider origins collapse into the canonical id whose metadata resolution actually uses.ModelsSectionfeeds it to the dropdown.SearchableModelSelectcaps rendering at 200 rows with a "+N more, keep typing to filter" footer so the popover stays responsive with the full catalog.Validation
make update-modelsrun twice: first refreshes, second prints "already up to date" with a clean tree.openai:gpt-4o-mini) saves and inherits 128k context, "None (use own metadata)" clears it, curated Claude mappings still load;make update-modelsidempotency re-verified in the workspace; no console errors. Search latency measured at 4-31 ms per keystroke.make lint-actions(actionlint + zizmor) clean for the new workflow.Risks
bun test srcpasses on the refreshed data; the main residual risk is upstream metadata quality for non-curated models, which the pricing sanitization bounds.Generated with
xum• Model:anthropic:claude-fable-5• Thinking:xhigh