fix: apply model metadata overrides to provider models#25
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a robust mechanism for applying user-defined model metadata overrides within the OmniRoute plugin. It adds logic to preserve raw user configurations across lifecycle hooks using internal markers and Symbols, ensuring that fetched models correctly reflect user-specified limits and features. The review feedback suggests minor refactorings to improve maintainability, specifically by simplifying redundant string comparisons in the matcher logic and using a more concise, loop-based approach for extracting metadata properties.
| if (typeof match === 'string') { | ||
| if (match === modelId) return true; | ||
|
|
||
| try { | ||
| return new RegExp(match).test(modelId); | ||
| } catch { | ||
| return false; | ||
| } | ||
| } |
There was a problem hiding this comment.
The if (match === modelId) return true; check is redundant as the calling function metadataBlockMatches already performs an exact string comparison before calling this function. You can remove this check to simplify the logic.
if (typeof match === 'string') {
try {
return new RegExp(match).test(modelId);
} catch {
return false;
}
}There was a problem hiding this comment.
Pull request overview
This PR updates the OmniRoute auth plugin so user-specified modelMetadata overrides are applied to fetched/default OmniRoute models before converting them into OpenCode provider models, while also preserving raw user-authored metadata across repeated hook runs and config cloning.
Changes:
- Apply user
modelMetadataoverrides to fetched/default models prior totoProviderModels()in config/provider/auth flows. - Persist “raw user modelMetadata” separately from plugin-generated metadata across hook runs (including JSON-clone scenarios).
- Refresh plugin-generated
provider.modelson repeated config hook runs while preserving explicit user-definedprovider.omniroute.models, with added regression tests.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/plugin.ts |
Applies model metadata overrides earlier in the pipeline; preserves raw user metadata and marks plugin-generated models for refresh/preservation logic. |
test/plugin.test.mjs |
Adds regression coverage for override application, alias canonicalization, addIfMissing, repeated hook runs, and JSON-cloned raw metadata behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Code Review Roast 🔥Verdict: 1 Issue Found | Recommendation: Address before merge Overview
Issue Details (click to expand)
🏆 Best part: Finally removing that dead 💀 Worst part: The 📊 Overall: Like a regex that's almost perfect but misses the dot - technically functional but not quite matching the spec. Files Reviewed (2 files)
|
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
Code Review Roast 🔥Verdict: No Issues Found | Recommendation: Merge Oh wait, this PR is actually clean. I had my flamethrower warmed up and everything. 📊 Overall: Like finding a unicorn in production — I didn't think clean PRs existed anymore, but here we are. Files Reviewed (2 files)
|
Code Review Roast 🔥Verdict: 1 Issue Found | Recommendation: Address before merge Overview
Issue Details (click to expand)
🏆 Best part: Removing that unused 💀 Worst part: The 📊 Overall: Like a regex that's almost perfect but misses the dot - technically functional but not quite matching the spec. Files Reviewed (2 files)
|
Code Review Roast 🔥Verdict: 1 Issue Found | Recommendation: Address before merge Overview
Issue Details (click to expand)
🏆 Best part: Removing that unused 💀 Worst part: The 📊 Overall: Like a regex that's almost perfect but misses the dot - technically functional but not quite matching the spec. Files Reviewed (2 files)
Fix these issues in Kilo Cloud Reviewed by nemotron-3-super-120b-a12b-20230311:free · 216,180 tokens |
Review Fixes AppliedPushed fixes to branch Issues Fixed
Verification
Commits
Ready for re-review. |
|
Update: All 4 fix commits have been pushed directly to this PR's source branch (). No separate branch needed — the commits are now part of this PR. |
|
Merge Conflict Resolved Merged release/v1.2.2 into this PR and resolved the conflict in test/plugin.test.mjs. The afterEach hook now correctly combines both:
All 66 tests passing. |
Brings in PR #25 fixes (model metadata overrides to provider models) alongside the local AGENTS.md consolidation and CLAUDE.md symlink.
…olidation (#26) * fix: apply model metadata overrides to provider models (#25) * fix: apply model metadata overrides to provider models * test: isolate auth store XDG data home * fix: treat string metadata matches as literals * refactor: centralize metadata extraction keys * fix: refresh legacy generated provider models --------- Co-authored-by: Sebastian Rumpf <alp4d0g007@googlemail.com> * docs: consolidate agent guidelines into AGENTS.md and symlink CLAUDE.md - Rewrite AGENTS.md by cherry-picking the best of AGENTS.md and CLAUDE.md: - Keep generic Agent Guidelines framing and all code style rules - Add Architecture section (dual entry points, core modules, fetch interceptor, caching) - Expand Common Commands (test runner, check:exports, single-file typecheck) - Add Testing, Important Configuration, and full Release Process sections - Delete CLAUDE.md and replace with symlink to AGENTS.md so both Claude Code and other agents read from the same canonical source. * chore: bump version to 1.2.2 * style: fix indentation in getApiMode warn() call Fixes pre-existing indentation issue flagged by kilo-code-bot review. No functional change. * perf: pre-process metadata blocks to avoid redundant regex/alias calculations Addresses gemini-code-assist review feedback on PR #26: - Pre-process array-based metadata blocks once: canonicalize string matches, compile regexes, and extract metadata upfront. - Replaces O(N*M) redundant resolveProviderAliasForMetadata + coerceRegExp calls inside per-model loops with a single pass over config blocks. - Adds processedBlockMatches() helper that uses pre-computed canonicalMatch instead of re-resolving aliases on every comparison. - No functional change — all 66 tests pass. --------- Co-authored-by: Alexander Bien <abien@gmx.net>
Summary
modelMetadataoverrides to fetched/default models before converting them to OpenCode provider modelsmodelMetadataseparate from plugin-generated metadata across config/provider/auth hook runsprovider.omniroute.modelsaddIfMissing, and JSON-cloned raw metadataReview note
This PR intentionally does not address regex/ReDoS behavior for
modelMetadata.match. Upstream already documents RegExp matchers and accepts regex-like match blocks; this change only starts evaluating those existing user-configured matchers inside the plugin while applying overrides beforetoProviderModels(). If maintainers want stricter regex safety, that should be handled as a separate behavior/security hardening change.Verification
npm testnpm run prepublishOnlygit diff --check upstream/main...HEAD