feat(kiro): add model group routing preferences#61
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces "Model Group Routing" for Kiro, allowing administrators to map specific models to preferred account groups. The changes span database migrations, Postgres repository updates, API endpoints, frontend UI configuration, and route selection logic to prioritize preferred accounts. The review feedback identifies a critical bug where inserting a column in the middle of a SELECT query shifts indices and breaks hardcoded database decoding. Additionally, suggestions are provided to improve robustness by gracefully falling back when configured account groups are missing or invalid, and to optimize route selection by only bypassing sticky affinity when the preferred account set is non-empty.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| k.created_at_ms, k.updated_at_ms, | ||
| r.route_strategy, r.fixed_account_name, r.auto_account_names_json, | ||
| r.account_group_id, r.model_name_map_json, | ||
| r.kiro_model_group_preferences_json, | ||
| r.request_max_concurrency, r.request_min_start_interval_ms, | ||
| r.codex_fast_enabled, r.codex_strict_session_rejection_enabled, | ||
| r.codex_image_generation_enabled, |
There was a problem hiding this comment.
Critical Bug: Column Index Shift in SELECT Query
In decode_key_bundle (defined in crates/llm-access-store/src/postgres/decode.rs), several columns are decoded using hardcoded indices (e.g., model_name_map_json at index 15, request_max_concurrency at index 16, request_min_start_interval_ms at index 17, and codex_fast_enabled at index 18).
By inserting r.kiro_model_group_preferences_json in the middle of the SELECT list (at line 352), you have shifted the indices of all subsequent columns by 1. This will cause decode_key_bundle to decode incorrect fields or fail with a database decoding error at runtime.
To fix this safely without breaking the hardcoded indices, move r.kiro_model_group_preferences_json to the end of the SELECT list (after r.codex_image_generation_enabled), matching the pattern used in the other queries in this file.
| k.created_at_ms, k.updated_at_ms, | |
| r.route_strategy, r.fixed_account_name, r.auto_account_names_json, | |
| r.account_group_id, r.model_name_map_json, | |
| r.kiro_model_group_preferences_json, | |
| r.request_max_concurrency, r.request_min_start_interval_ms, | |
| r.codex_fast_enabled, r.codex_strict_session_rejection_enabled, | |
| r.codex_image_generation_enabled, | |
| k.created_at_ms, k.updated_at_ms, | |
| r.route_strategy, r.fixed_account_name, r.auto_account_names_json, | |
| r.account_group_id, r.model_name_map_json, | |
| r.request_max_concurrency, r.request_min_start_interval_ms, | |
| r.codex_fast_enabled, r.codex_strict_session_rejection_enabled, | |
| r.codex_image_generation_enabled, | |
| r.kiro_model_group_preferences_json |
| let group = self | ||
| .get_admin_account_group_row(&group_id) | ||
| .await? | ||
| .with_context(|| { | ||
| format!("configured kiro model group preference `{group_id}` does not exist") | ||
| })?; | ||
| if group.provider_type != core_store::PROVIDER_KIRO { | ||
| anyhow::bail!( | ||
| "configured kiro model group preference belongs to a different provider" | ||
| ); | ||
| } |
There was a problem hiding this comment.
Robustness Issue: Graceful Fallback for Missing/Invalid Account Groups
Currently, if an account group configured in kiro_model_group_preferences is deleted or belongs to a different provider, resolve_kiro_model_group_preferred_account_names will return an Err (via anyhow::bail! or with_context). This error bubbles up and causes the entire key's route resolution or cache snapshot generation to fail, making the key completely unusable.
To make the routing more robust, log a warning and skip the invalid preference instead of failing the entire operation. This ensures that other models or fallback routing still work even if one preference is misconfigured or pointing to a deleted group.
let group = match self.get_admin_account_group_row(&group_id).await {
Ok(Some(g)) => g,
Ok(None) => {
tracing::warn!(
group_id = %group_id,
model = %model,
"Configured Kiro model group preference does not exist, skipping"
);
continue;
}
Err(err) => {
tracing::error!(
group_id = %group_id,
model = %model,
?err,
"Failed to load Kiro model group preference, skipping"
);
continue;
}
};
if group.provider_type != core_store::PROVIDER_KIRO {
tracing::warn!(
group_id = %group_id,
model = %model,
provider_type = %group.provider_type,
"Configured Kiro model group preference belongs to a different provider, skipping"
);
continue;
}| "kiro route is not configured", | ||
| )); | ||
| } | ||
| let model_preference_active = model_preferred_account_names.is_some(); |
There was a problem hiding this comment.
Optimization: Only Ignore Sticky Affinity if Preferred Accounts are Available
Currently, model_preference_active is set to true as long as model_preferred_account_names is Some, even if the set of preferred accounts is empty (e.g., because the preferred group has no accounts or none of them overlap with the key's authorized accounts). This causes sticky affinity to be ignored, degrading session consistency and cache hit rates for no benefit since no preferred accounts can be selected anyway.
We should only treat the model preference as active if the preferred account set is non-empty.
| let model_preference_active = model_preferred_account_names.is_some(); | |
| let model_preference_active = model_preferred_account_names.is_some_and(|set| !set.is_empty()); |
Summary
Add per-key Kiro model-to-account-group routing preferences so selected models can prefer a specific Kiro account group without expanding the main key editor card.
Root cause
Kiro routing only considered session affinity, route scope, pool strategy, latency, and local limits. There was no first-class way to bias a specific requested model toward a dedicated account group, and adding that inline to the existing admin card would make the Kiro key editor too long and heavier to render.
What changed
kiro_model_group_preferencesto key config, Postgres schema/migrations, request cache snapshots, and route resolution.Verification
cargo test -p llm-access-core -p llm-access-store -p llm-access --jobs 4cargo test -p llm-access kiro_selection_model_group_preference --jobs 4 -- --nocapturecargo check -p static-flow-frontend --target wasm32-unknown-unknown --jobs 4cargo clippy -p llm-access-core -p llm-access-store -p llm-access --jobs 4 -- -D warningscargo clippy -p static-flow-frontend --target wasm32-unknown-unknown --jobs 4 -- -D warningsDeployment note
Cloud
llm-accessneeds the new Postgres migration before rolling out this API path.