refactor: promote KeyAliases values from string to AliasConfig with backward-compatible JSON marshaling - #4014
Merged
akshaydeo merged 1 commit intoJun 9, 2026
Conversation
|
|
Contributor
|
Warning Review limit reached
More reviews will be available in 27 minutes and 35 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (24)
📝 WalkthroughWalkthroughRefactors KeyAliases from string values to rich AliasConfig objects with backward-compatible JSON; propagates the type through provider list-model signatures, the ListModelsPipeline, OpenRouter/Vertex helpers, transport fixtures, and persistence/encryption/migration tests. ChangesKeyAliases Schema and Compatibility
Provider Method Signature & Pipeline Updates
Test Data, Fixtures, and Transport Tests
Persistence, Encryption, Migration, and Hash Tests
🎯 4 (Complex) | ⏱️ ~60 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
This was referenced Jun 3, 2026
Collaborator
Author
This was referenced Jun 3, 2026
Merged
Merged
Contributor
Confidence Score: 5/5Safe to merge. The backward-compatible JSON marshaling keeps the legacy wire format byte-stable for unenriched entries, and DB persistence tests confirm round-trip correctness for both legacy and rich alias shapes. The refactor is well-scoped: custom MarshalJSON/UnmarshalJSON correctly handles both shapes, the type alias trick prevents infinite recursion, all provider call sites are updated consistently, and the schema update is complete. The test suite covers marshal/unmarshal round-trips, hash stability, DB encryption/decryption, and Validate edge cases. core/schemas/account.go — three items noted in earlier threads (VLLMAliasCfg dead code, empty-string ModelName pointer validation gap, empty embedded sub-config pointer round-trip contract) remain open but do not block correctness of the current change. Important Files Changed
Reviews (10): Last reviewed commit: "feat: extend key aliases to support depl..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Pratham-Mishra04
force-pushed
the
06-01-feat_add_governance_allowlist_support_for_all_provider_routing
branch
from
June 3, 2026 21:47
b03005e to
e0a8ea4
Compare
Pratham-Mishra04
force-pushed
the
06-02-feat_extend_key_aliases_to_support_deployment_level_configurations
branch
from
June 3, 2026 21:47
5e26e66 to
71fedcc
Compare
Pratham-Mishra04
force-pushed
the
06-02-feat_extend_key_aliases_to_support_deployment_level_configurations
branch
from
June 4, 2026 20:49
71fedcc to
f407b57
Compare
Pratham-Mishra04
force-pushed
the
06-01-feat_add_governance_allowlist_support_for_all_provider_routing
branch
from
June 4, 2026 20:49
e0a8ea4 to
c9d3c23
Compare
18 tasks
Pratham-Mishra04
force-pushed
the
06-02-feat_extend_key_aliases_to_support_deployment_level_configurations
branch
from
June 5, 2026 09:48
f407b57 to
c83f2c6
Compare
Pratham-Mishra04
force-pushed
the
06-01-feat_add_governance_allowlist_support_for_all_provider_routing
branch
from
June 5, 2026 09:48
c9d3c23 to
cd5bdec
Compare
Pratham-Mishra04
force-pushed
the
06-02-feat_extend_key_aliases_to_support_deployment_level_configurations
branch
from
June 7, 2026 07:25
c83f2c6 to
4439bc5
Compare
Pratham-Mishra04
force-pushed
the
06-01-feat_add_governance_allowlist_support_for_all_provider_routing
branch
2 times, most recently
from
June 8, 2026 06:54
694970e to
a2384cb
Compare
Pratham-Mishra04
force-pushed
the
06-02-feat_extend_key_aliases_to_support_deployment_level_configurations
branch
from
June 8, 2026 06:55
4439bc5 to
6a64d6d
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
framework/configstore/tables/encryption_test.go (1)
1969-2022:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAssert mixed alias wire shape before normalization.
Line 2017 checks only post-unmarshal values, so it won’t catch a regression where
"plain"is persisted as an object instead of a legacy string when a rich sibling exists. Add a rawaliases_jsondecrypt assertion in this test beforedb.First(...).As per coding guidelines,
framework/**changes should preserve backward-compatible data formats and include tests that cover edge cases and failure paths.Proposed test hardening
func TestTableKey_AliasesJSON_RichRoundTrip(t *testing.T) { db := setupTestDB(t) @@ } require.NoError(t, db.Create(key).Error) + + // Assert raw persisted mixed shape before AfterFind normalization: + // rich entry should be object form, plain sibling should remain legacy string form. + raw := rawRow(t, db, "config_keys", key.ID) + rawAliasesVal := raw["aliases_json"] + var rawAliasesStr string + switch v := rawAliasesVal.(type) { + case string: + rawAliasesStr = v + case []byte: + rawAliasesStr = string(v) + } + require.NotEmpty(t, rawAliasesStr) + plaintext, err := encrypt.Decrypt(rawAliasesStr) + require.NoError(t, err) + assert.Contains(t, plaintext, `"plain":"gpt-4o-fallback"`) + assert.Contains(t, plaintext, `"best-model":{"model_id":"azure-deployment-xyz"`) var found TableKey require.NoError(t, db.First(&found, key.ID).Error)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@framework/configstore/tables/encryption_test.go` around lines 1969 - 2022, Add a pre-unmarshal assertion that the on-disk column "aliases_json" for the inserted TableKey remains the legacy mixed wire-shape (i.e., "plain" persisted as a string) before calling db.First/auto-unmarshal: after creating key (created via setupTestDB and db.Create(key)), fetch the raw aliases_json for key.ID (e.g., via db.Model(&TableKey{}).Select("aliases_json").Where("id = ?", key.ID) or equivalent), decrypt/unwrap that raw payload using the same aliases decryption helper used by TableKey (or a decryptAliasesJSON helper), and assert the decrypted JSON still contains "plain" as a string (not an object) while "best-model" is the rich object; then proceed with the existing db.First(&found, key.ID) and the rest of the assertions.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@core/providers/openrouter/openrouter.go`:
- Around line 209-214: The alias normalization silently overwrites entries when
stripPrefix(k) collides (e.g., "foo" vs "openrouter/foo") in the loop over
key.Aliases; update the logic that populates normalizedAliases so collisions are
handled deterministically: iterate the alias map keys in a sorted order (or
otherwise deterministic order), compute n := stripPrefix(k) and if
normalizedAliases already has n either (a) apply a clear precedence rule (e.g.,
prefer the exact key name over the prefixed one or vice versa) or (b) keep the
first-seen entry and log or return an error indicating the collision; change
code around normalizedAliases, stripPrefix, and the loop over key.Aliases to
implement this deterministic collision resolution.
---
Duplicate comments:
In `@framework/configstore/tables/encryption_test.go`:
- Around line 1969-2022: Add a pre-unmarshal assertion that the on-disk column
"aliases_json" for the inserted TableKey remains the legacy mixed wire-shape
(i.e., "plain" persisted as a string) before calling db.First/auto-unmarshal:
after creating key (created via setupTestDB and db.Create(key)), fetch the raw
aliases_json for key.ID (e.g., via
db.Model(&TableKey{}).Select("aliases_json").Where("id = ?", key.ID) or
equivalent), decrypt/unwrap that raw payload using the same aliases decryption
helper used by TableKey (or a decryptAliasesJSON helper), and assert the
decrypted JSON still contains "plain" as a string (not an object) while
"best-model" is the rich object; then proceed with the existing db.First(&found,
key.ID) and the rest of the assertions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 670375fa-b697-436f-861a-5343de276f3b
📒 Files selected for processing (22)
core/internal/llmtests/account.gocore/providers/anthropic/models.gocore/providers/azure/models.gocore/providers/bedrock/models.gocore/providers/bedrock/rerank_test.gocore/providers/cohere/models.gocore/providers/elevenlabs/models.gocore/providers/gemini/models.gocore/providers/huggingface/models.gocore/providers/mistral/models.gocore/providers/openai/models.gocore/providers/openrouter/openrouter.gocore/providers/replicate/models.gocore/providers/utils/models.gocore/providers/vertex/models.gocore/providers/vertex/utils.gocore/schemas/account.gocore/schemas/account_test.goframework/configstore/encryption_test.goframework/configstore/keyhash_alias_test.goframework/configstore/migrations_test.goframework/configstore/tables/encryption_test.go
Pratham-Mishra04
force-pushed
the
06-01-feat_add_governance_allowlist_support_for_all_provider_routing
branch
from
June 8, 2026 07:18
a2384cb to
c0c727c
Compare
Pratham-Mishra04
force-pushed
the
06-02-feat_extend_key_aliases_to_support_deployment_level_configurations
branch
from
June 8, 2026 07:18
6a64d6d to
d18d03a
Compare
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
transports/config.schema.json (1)
2403-2413:⚠️ Potential issue | 🟠 Major | ⚡ Quick winTighten alias whitespace validation to match runtime contract.
Line 2403 and Line 2410 currently accept values with leading/trailing whitespace, and Line 2465 allows whitespace-padded alias keys. Runtime
KeyAliases.Validate()rejects these, so malformed configs pass schema validation and then fail later at load/validation time. Please enforce no leading/trailing whitespace in the legacy string branch,model_id, and aliaspropertyNames(and keepmodel_namealigned with runtime whitespace rules too).As per coding guidelines,
transports/config.schema.jsonis the source of truth for config fields, so its constraints should match runtime validation behavior exactly.Also applies to: 2465-2467
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@transports/config.schema.json` around lines 2403 - 2413, The schema currently allows leading/trailing whitespace for the legacy alias string, model_id, model_name, and alias property names, which diverges from runtime KeyAliases.Validate(); update the schema to forbid surrounding whitespace: add a pattern like "^(?!\\s).*(?<!\\s)$" (or equivalent) to the legacy string branch, to the "model_id" and "model_name" property definitions, and add the same pattern under the alias object’s "propertyNames" to prevent whitespace-padded keys so schema validation matches KeyAliases.Validate().Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@transports/config.schema.json`:
- Around line 2403-2413: The schema currently allows leading/trailing whitespace
for the legacy alias string, model_id, model_name, and alias property names,
which diverges from runtime KeyAliases.Validate(); update the schema to forbid
surrounding whitespace: add a pattern like "^(?!\\s).*(?<!\\s)$" (or equivalent)
to the legacy string branch, to the "model_id" and "model_name" property
definitions, and add the same pattern under the alias object’s "propertyNames"
to prevent whitespace-padded keys so schema validation matches
KeyAliases.Validate().
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: d40bb20f-c259-443c-9c53-5240f44db413
📒 Files selected for processing (24)
core/internal/llmtests/account.gocore/providers/anthropic/models.gocore/providers/azure/models.gocore/providers/bedrock/models.gocore/providers/bedrock/rerank_test.gocore/providers/cohere/models.gocore/providers/elevenlabs/models.gocore/providers/gemini/models.gocore/providers/huggingface/models.gocore/providers/mistral/models.gocore/providers/openai/models.gocore/providers/openrouter/openrouter.gocore/providers/replicate/models.gocore/providers/utils/models.gocore/providers/vertex/models.gocore/providers/vertex/utils.gocore/schemas/account.gocore/schemas/account_test.goframework/configstore/encryption_test.goframework/configstore/keyhash_alias_test.goframework/configstore/migrations_test.goframework/configstore/tables/encryption_test.gotransports/bifrost-http/lib/config_test.gotransports/config.schema.json
Pratham-Mishra04
force-pushed
the
06-02-feat_extend_key_aliases_to_support_deployment_level_configurations
branch
from
June 8, 2026 11:55
d18d03a to
e82bb5b
Compare
Pratham-Mishra04
force-pushed
the
06-01-feat_add_governance_allowlist_support_for_all_provider_routing
branch
from
June 8, 2026 11:55
c0c727c to
c6758e2
Compare
Pratham-Mishra04
force-pushed
the
06-02-feat_extend_key_aliases_to_support_deployment_level_configurations
branch
from
June 8, 2026 12:24
e82bb5b to
cc297cb
Compare
Madhuvod
changed the base branch from
06-01-feat_add_governance_allowlist_support_for_all_provider_routing
to
graphite-base/4014
June 8, 2026 12:25
Pratham-Mishra04
force-pushed
the
graphite-base/4014
branch
from
June 8, 2026 12:28
0cb1e9f to
8be1116
Compare
Pratham-Mishra04
force-pushed
the
06-02-feat_extend_key_aliases_to_support_deployment_level_configurations
branch
from
June 8, 2026 12:28
cc297cb to
1a04ffb
Compare
Pratham-Mishra04
changed the base branch from
graphite-base/4014
to
06-01-feat_add_governance_allowlist_support_for_all_provider_routing
June 8, 2026 12:28
18 tasks
Contributor
Merge activity
|
akshaydeo
changed the base branch from
06-01-feat_add_governance_allowlist_support_for_all_provider_routing
to
graphite-base/4014
June 9, 2026 05:22
akshaydeo
deleted the
06-02-feat_extend_key_aliases_to_support_deployment_level_configurations
branch
June 9, 2026 05:23
This was referenced Jun 9, 2026
Merged
18 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
KeyAliasespreviously mapped user-facing model names to plain strings (map[string]string). This PR promotes the value type to a richAliasConfigstruct that carries the wire model identifier (ModelID), an optional canonical model name (ModelName), a typed model family enum (ModelFamily), and optional provider-specific override sub-configs (AzureAliasCfg,VertexAliasCfg,BedrockAliasCfg,ReplicateAliasCfg). The change lays the groundwork for provider routing decisions and per-alias overrides without substring-sniffing wire model IDs.Changes
KeyAliasesis nowmap[string]AliasConfiginstead ofmap[string]string. A customUnmarshalJSONtransparently promotes legacy string values ("my-model": "provider-id") toAliasConfig{ModelID: "provider-id"}, andAliasConfig.MarshalJSONemits the legacy string wire shape when onlyModelIDis set — keeping the JSON wire format andconfig_hashbyte-stable for unenriched entries.ModelFamilytyped enum (anthropic,openai,mistral,cohere,gemini,nova,titan) with anIsValid()method for validation.AzureAliasCfg(api_version, anthropic_version, endpoint),VertexAliasCfg(project_id, project_number),BedrockAliasCfg(inference_profile_arn),ReplicateAliasCfg(use_deployments_endpoint).KeyAliases.Resolveis preserved for backward compatibility; a newResolveConfigmethod returns the fullAliasConfig.KeyAliases.Validateextended to checkModelIDemptiness/whitespace,ModelNamewhitespace, andModelFamilyvalidity.ToBifrostListModelsResponsesignatures updated frommap[string]stringtoschemas.KeyAliases, with internal alias iteration updated to readalias.ModelID.ListModelsPipeline.Aliasesfield updated toschemas.KeyAliases;resolveModelIDandBackfillModelsupdated accordingly.listModelsByKeyupdated to copy and strip prefixes fromAliasConfig.ModelIDrather than the raw string.config.schema.jsonupdated so alias values accept either the legacy string shape or the new object shape viaoneOf.KeyAliasesmarshal/unmarshal round-trips (both legacy and rich shapes),ResolveConfig,Validate,ModelFamily.IsValid, DB persistence of legacy and rich alias shapes, andGenerateKeyHashstability.Type of change
Affected areas
How to test
Verify that existing configs using the legacy
"alias": "model-id"string shape continue to load and hash identically. Verify that a config using the new object shape"alias": {"model_id": "model-id", "model_family": "anthropic"}loads correctly and produces a different hash than the unenriched equivalent.Breaking changes
Any code that directly indexes into
KeyAliasesasmap[string]string(e.g.,aliases["key"]expecting astring) must be updated to access.ModelIDon the returnedAliasConfig. All in-repo call sites have been updated. External consumers constructingKeyAliasesliterals must change{"k": "v"}to{"k": {ModelID: "v"}}in Go code; JSON configs require no change due to the transparent legacy deserialization.Related issues
Security considerations
No new secrets or auth surfaces introduced. The alias sub-configs (
AzureAliasCfg.Endpoint,VertexAliasCfg.ProjectID, etc.) may referenceEnvVarvalues; these follow the same encryption path as existing key-level fields.Checklist
docs/contributing/README.mdand followed the guidelinesSummary by CodeRabbit
Refactor
Tests