Repository navigation
Conversation
Preserve the reviewed AISDLC-95 refinement artifacts. Link the research from a consolidated implementation plan. Record Jira traceability and ADR constraints. Document reporter decision gates before the delivery breakdown. Signed-off-by: Tim Pouyer <tim.pouyer@gmail.com>
E2E tests did not runE2E tests run automatically for org/repo members and collaborators on pull requests. For other contributors, a maintainer with write access must add the See E2E testing guide for details. |
PR Summary by QodoDocument AISDLC-95 model-routing research and proposed delivery plan
AI Description
Diagram
High-Level Assessment
Files changed (17)
|
Code Review by Qodo
1. A denied choice can reach an unapproved model
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
| The recommended approach separates model routing into three responsibilities: | ||
|
|
||
| 1. **Learn from completed work without changing production automatically.** | ||
| An offline process studies quality, cost, latency, and completion evidence. | ||
| It may propose a new routing policy, but people must review and approve the | ||
| exact policy before it can be used. |
There was a problem hiding this comment.
2. Proposed routing lacks a planned callout 📜 Skill insight ≡ Correctness
The new README presents automatic model selection, offline policy, and gateway serving as proposed features in ordinary paragraphs and numbered content, without a > **Planned:** callout containing an issue link. Readers scanning the spike can encounter implementation-shaped behavior without the standardized planned-feature marker, while the surrounding disclaimer does not satisfy that required format.
Agent Prompt
## Issue description
The documentation describes automatic model routing as a proposed feature but does not use the required `> **Planned:**` blockquote callout with a relevant issue link.
## Fix Focus Areas
- docs/spikes/aisdlc-95/README.md[15-28]
## Recommended Fix
Add a `> **Planned:**` callout near the first description of the proposed routing behavior, including a link to the relevant AISDLC issue. Apply the same format to other newly added documents where not-yet-implemented routing features are introduced.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| if denied: | ||
| if sameRouteFallback exists: | ||
| return PlanConcreteOnSameGatewayRoute(sameRouteFallback) |
There was a problem hiding this comment.
1. A denied choice can reach an unapproved model 🐞 Bug ⛨ Security
PlanConcreteGatewayRun validates the effective model against run.approvedProviderPolicy, but returns sameRouteFallback after denial without applying that policy or checking gateway authorization for the fallback. When the trusted fallback configuration permits a model outside the run's approved set, the serving plan can select it even though staying on the same gateway block and authentication mode does not establish model approval.
Agent Prompt
## Issue description
The serving-layer prototype can plan a configured fallback after denying the effective model without validating that fallback against the run's provider policy or gateway authorization.
## Fix Focus Areas
- docs/spikes/aisdlc-95/spikes/AISDLC-289/prototype.md[308-331]
## Recommended Fix
Before returning a fallback plan, apply the same concrete-model policy gate to the fallback and verify that the configured gateway authorizes its canonical ID for the repository; otherwise fail closed.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| if err != nil { return err } | ||
| route, err := startGatewayRoute(ctx, plan) | ||
| if err != nil { return err } | ||
| // Any later control-plane, credential, gateway-side classifier, | ||
| // gateway, or upstream failure returns error. Never try direct | ||
| // OpenAI/Vertex/static key and never switch oidc <-> api-key. |
There was a problem hiding this comment.
3. Setup failures leave no terminal outcome 🐞 Bug ◔ Observability
RunAgent writes a decision before side effects, but writes RecordRunOutcome only after route setup and runtime execution. A fatal gate rejection, gateway planning error, startup error, or cancellation during setup returns before that record is written, despite the prototype's requirement for one terminal decision/outcome record on cancellation paths.
Agent Prompt
## Issue description
The runner pseudocode returns from several post-decision failure paths without recording a terminal outcome.
## Fix Focus Areas
- docs/spikes/aisdlc-95/spikes/AISDLC-291/prototype.md[436-479]
- docs/spikes/aisdlc-95/spikes/AISDLC-291/prototype.md[610-615]
## Recommended Fix
Define one terminal-outcome recording path for every exit after a decision is created, including gate rejection, route planning, startup, and cancellation. Record unavailable metrics and served-model evidence as unknown rather than inventing values.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Site previewPreview: https://cd824d99-site.fullsend-ai.workers.dev Commit: |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
waynesun09
left a comment
waynesun09
left a comment
There was a problem hiding this comment.
Review comments: three MEDIUM findings inline.
| for the advisor application point and gateway-selector control plane. | ||
| - Re-read current Jira and repository state before creating delivery work. | ||
|
|
||
| Exit gate: the reporter approves the planning basis and every unresolved item |
There was a problem hiding this comment.
MEDIUM: Spike-level open decisions, including whether the served-identity evidence for promotion and compliance is attainable, are left out of the D1-D12 gate
The README (lines 57, 67, 86, 116, 206, 305) and Phase 0 (line 141) send reviewers, including the AI-facilitator instructions, through exactly twelve decisions (D1-D12). The spike documents list more open human decisions that D1-D12 does not cover:
- AISDLC-289/research.md: Add problem areas: Tekton pipeline review, migration path, multi-tenancy #2, the routing unit and whether session pinning is mandatory; Add CODEOWNERS #6, which providers expose signed metadata or independently retrievable audit records; docs: Add contributor guidance problem domain #7, who owns routing-policy lifecycle and rollback; docs: add pipeline-execution-feedback problem doc #8, whether ADR 0138 api-key mode is eligible for the first pilot.
- AISDLC-291/research.md: Add some harness engineering posts to the backlog #10, whether the advisor is local or remote and what data may leave the runner; Add dual-interpretation escalation pattern for agent-to-human handoffs #11, served-model verification for runtimes that do not report it; Add problem document for strategic risks #13, how
FULLSEND_FALLBACK_MODELSis constrained after an advisor selection. - AISDLC-290/research.md: docs: add pipeline-execution-feedback problem doc #8, the signing trust and approval registry.
#6 has the largest effect. AISDLC-289/research.md:319-320 and AISDLC-290/research.md:152-154 require provider-attested or independently-reconciled evidence for provider/data-policy compliance, and independently-reconciled evidence for any promotion based on served, provider or cost claims. The PR does not show that any provider or gateway deployment produces that evidence today. On the ADR 0137 route the gateway holds the upstream key. Even so, Phase 3's exit and Phase 6 ("Prove actual-served identity at the assurance required for each claim") present the gate as one that can be met. The Phase 0 exit gate ("every unresolved item has a decision, owner or explicit deferral") does not say whether these spike items are included, so a review that follows the README can finish with them unresolved.
Suggestion: Map each spike-level unresolved question to a D-number, or add an "Additional open questions carried from spikes" table to implementation-plan.md with an owner/deferral column, and make the README review process cover it. Make the attainability of provider-attested or independently-reconciled evidence (AISDLC-289 #6) an explicit reporter decision. If no current provider or gateway can produce it, state that gateway-route promotion depends on the residual-risk exception path.
| RequestedRoute "direct" | "gateway" | ||
| GatewayAuthMode Optional["oidc" | "api-key"] // never credential value | ||
| ServedModel Optional[string] | ||
| ServedVerification "provider_reported" | "gateway_reported" | |
There was a problem hiding this comment.
MEDIUM: The advisor prototype's ServedVerification enum conflicts with the five-value assurance scale the other spikes use
RoutingDecisionRecord.ServedVerification (lines 547-548) has the values provider_reported | gateway_reported | requested_only | unknown. AISDLC-289/research.md:302-320, AISDLC-290/research.md:141-154 and AISDLC-290/prototype.md:86-87 instead define Assurance as requested-only | runtime-reported | gateway-attested | provider-attested | independently-reconciled, and the promotion and compliance gates are written against that scale. The advisor record emits served-identity evidence, but it cannot express runtime-reported or independently-reconciled (which promotion requires), and its spelling and terms differ ("reported" versus "attested", underscores versus hyphens). The consolidated plan's Phase 2 (implementation-plan.md:166) requires "assurance" on identity evidence but defines no enum. Delivery teams following each spike would therefore build records that cannot be compared with each other.
Suggestion: Use the five-value AISDLC-289/290 assurance scale in the AISDLC-291 prototype (rename ServedVerification to Assurance with the same values), and define the enum once in implementation-plan.md Phase 2 so that every spike refers to that definition.
|
|
||
| LiteLLM's official beta Auto Routing documentation demonstrates that a gateway | ||
| can expose a normal model alias whose implementation classifies each request. | ||
| It supports heuristic, LLM, JEV, keyword, semantic, and custom classifiers; |
There was a problem hiding this comment.
MEDIUM: The LiteLLM Auto Routing capabilities are misstated against the cited primary source
Line 121 says LiteLLM "supports heuristic, LLM, JEV, keyword, semantic, and custom classifiers". The cited page (docs.litellm.ai/docs/proxy/auto_routing) documents the classifier_type values llm, jev and custom, with a heuristic scorer as the default. Keyword matching is a deterministic keyword_tier_rules short-circuit, semantic matching is a semantic_keyword_matching flag on those rules, and the standalone semantic router is deprecated.
Lines 131-133 say return_raw_model_name: false lets an alias "conceal the selected model from the client response". The page says that with the default the body's model field is restamped to the alias but the tier that answered "is reachable only through the x-litellm-model-id response header". The selected model is therefore still returned in a header. That changes the identity-evidence analysis: the evidence exists, as presentation-level data in the gateway trust domain.
Suggestion: Correct the classifier list to the documented classifier types and describe keyword and semantic matching as rule features. Restate the return_raw_model_name implication: the selected identity is still returned in the x-litellm-model-id header, and it counts only as gateway-trust-domain evidence (gateway-attested at most), not independent proof.
|
I see somewhere that is not available to the public that you do not intend to commit this. Can you add this to the PR description and move it to draft? I thought you wanted to commit this, which made no sense to me to commit it here (maybe on experiments, but not here). |
|
Closing this research proposal because the implementation work is now tracked under the AISDLC-95 feature:
This PR remains available as background research for those epics. |
Summary
Add the research and proposed implementation plan for AISDLC-95 automatic model routing. The documents distinguish current behavior and accepted architecture from recommendations that still require reporter approval.
Ask your AI to review this implementation plan by reading the How to review this proposal section in the docs/spikes/aisdlc-95/README.md
Related Issue
Changes
Testing
make lintpasses (stage changes first, then run)Checklist
!for breaking changes)