Skip to content

Handle models missing from the pricing table #66552

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Copilot wants to merge 7 commits into from
Closed

Handle models missing from the pricing table #66552

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
e5fbb5a
Initial plan
Copilot Oct 7, 2026
2732e85
Inject model pricing and warn on misses
Copilot Oct 7, 2026
c4751e5
docs: add draft ADR for compile-time model pricing injection
github-actions[bot] Oct 7, 2026
89f9659
Regenerate workflow pricing overlays
Copilot Oct 7, 2026
46f8220
Merge remote-tracking branch 'origin/main' into copilot/aw-top-10-han…
Copilot Oct 7, 2026
b2742c0
Fix model pricing review findings
Copilot Oct 7, 2026
2a75a62
Merge remote-tracking branch 'origin/main' into copilot/aw-top-10-han…
Copilot Oct 7, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Fix model pricing review findings 
Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
  • Loading branch information
@gh-aw-bot
Copilot and gh-aw-bot authored Oct 7, 2026
commit b2742c01bfd3a3406f23c3537c7f55a3d8dba20b
4 changes: 2 additions & 2 deletions pkg/cli/compile_compiler_setup.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -265,8 +265,8 @@ func setupRepositoryContext(compiler *workflow.Compiler, config CompileConfig) {
// If a schedule seed is explicitly provided, use it directly
if config.ScheduleSeed != "" {
// Validate owner/repo format: must contain exactly one '/' with non-empty parts
parts := strings.SplitN(config.ScheduleSeed, "/", 2)
if len(parts) != 2 || parts[0] == "" || parts[1] == "" {
owner, repo, hasSeparator := strings.Cut(config.ScheduleSeed, "/")
if !hasSeparator || owner == "" || repo == "" || strings.Contains(repo, "/") {
compileCompilerSetupLog.Printf("Invalid --schedule-seed value %q: expected 'owner/repo' format", config.ScheduleSeed)
fmt.Fprintln(os.Stderr, console.FormatWarningMessageStderr(
fmt.Sprintf("--schedule-seed %q is not in 'owner/repo' format; ignoring and falling back to git remote detection", config.ScheduleSeed),
Expand Down
50 changes: 50 additions & 0 deletions pkg/cli/compile_development_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,20 @@ func TestConfiguredModelPricingWarning(t *testing.T) {
ModelMappings: map[string][]string{"preferred-model": {"gpt-6.1-sol"}},
},
},
{
name: "builtin sonnet alias",
data: &workflow.WorkflowData{
Model: "sonnet",
ModelMappings: workflow.BuiltinModelAliases(),
},
},
{
name: "builtin haiku alias",
data: &workflow.WorkflowData{
Model: "haiku",
ModelMappings: workflow.BuiltinModelAliases(),
},
},
{
name: "configured fallback",
data: &workflow.WorkflowData{
Expand All @@ -158,6 +172,42 @@ func TestConfiguredModelPricingWarning(t *testing.T) {
},
},
},
{
name: "empty model entry does not count as pricing",
data: &workflow.WorkflowData{
Model: "gpt-future-model",
EngineConfig: &workflow.EngineConfig{ID: "codex"},
ModelCosts: map[string]any{
"providers": map[string]any{
"openai": map[string]any{
"models": map[string]any{
"gpt-future-model": map[string]any{},
},
},
},
},
},
warning: true,
},
{
name: "empty cost entry does not count as pricing",
data: &workflow.WorkflowData{
Model: "gpt-future-model",
EngineConfig: &workflow.EngineConfig{ID: "codex"},
ModelCosts: map[string]any{
"providers": map[string]any{
"openai": map[string]any{
"models": map[string]any{
"gpt-future-model": map[string]any{
"cost": map[string]any{},
},
},
},
},
},
},
warning: true,
},
{
name: "dynamic model",
data: &workflow.WorkflowData{
Expand Down
25 changes: 24 additions & 1 deletion pkg/cli/compile_model_validation.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,11 @@ func configuredModelHasPricing(provider, model string, aliases map[string][]stri
targetProvider, targetModel, ok := configuredModelPricingID(&workflow.WorkflowData{
EngineConfig: &workflow.EngineConfig{LLMProvider: workflow.LLMProvider(provider)},
}, target)

@github-actions github-actions Bot Oct 7, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alias glob patterns break configuredModelHasPricing and emit false-positive warnings for every built-in model alias.

Built-in aliases such as sonnet and haiku resolve to glob targets like copilot/*sonnet* (see pkg/workflow/model_aliases.go), but configuredModelPricingID (and the exact-match lookup it feeds into) treats each target as a literal model ID. A glob pattern never matches any catalog record, so configuredModelHasPricing returns false and the new warning fires for every ordinary workflow using a built-in alias, even though AWF resolves these at runtime.

Reproduced locally:

aliases := workflow.MergeImportedModelAliases(nil, nil)
data := &workflow.WorkflowData{Model: "sonnet", EngineConfig: &workflow.EngineConfig{ID: "copilot"}, ModelMappings: aliases}
configuredModelPricingWarning(data) // => "Model sonnet has no AI credits pricing..."

This fires spuriously for any workflow with model: sonnet or model: haiku, which is likely very common.

Please either skip/short-circuit glob targets (treat them as "has pricing" since AWF resolves them dynamically) or expand the glob against the catalog before declaring no pricing. @copilot please address this.

Copilot AI Oct 7, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Handled in b2742c01: alias glob targets are matched against priced catalog models, and built-in sonnet/haiku cases are covered by tests.

if !ok || !configuredModelHasPricing(targetProvider, targetModel, aliases, visited) {
hasPricing := ok && configuredModelHasPricing(targetProvider, targetModel, aliases, visited)
if ok && strings.ContainsAny(targetModel, "*[") {
hasPricing = hasCatalogPricingMatching(targetProvider, targetModel)
}
if !hasPricing {
delete(visited, alias)
return false
}
Expand Down Expand Up @@ -255,6 +259,14 @@ func hasModelCostOverlay(costs map[string]any, provider, model string) bool {
continue
}
for name := range models {
entry, ok := models[name].(map[string]any)
if !ok {
continue
}
cost, ok := entry["cost"]
if !ok || !hasNonEmptyCostMap(cost) {
continue
}
if strings.EqualFold(name, model) {
return true
Comment on lines +261 to +271

Copilot AI Oct 7, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Handled in b2742c01: both compiler and workflow pricing predicates now require a non-empty cost map; regression tests cover absent and empty pricing entries.

@github-actions github-actions Bot Oct 7, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hasModelCostOverlay returns true for an entry with no cost field, suppressing the new pricing warning incorrectly.

The schema permits a frontmatter model entry such as models.providers.openai.models.custom: {} (no cost key, or an empty cost map). hasModelCostOverlay only checks that the model key exists under providerMap["models"], not that it actually carries pricing data:

costs := map[string]any{"providers": map[string]any{"openai": map[string]any{"models": map[string]any{"custom-model": map[string]any{}}}}}
hasModelCostOverlay(costs, "openai", "custom-model") // => true (should be false — no cost present)

This means a frontmatter stub entry with no actual price silently suppresses configuredModelPricingWarning, defeating the purpose of this PR (catching models the AWF proxy will reject with HTTP 400). Check that cost is present and non-empty before returning true. @copilot please address this.

Copilot AI Oct 7, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Handled in b2742c01: an overlay only counts as pricing when the model has a non-empty cost map, covered for missing and empty maps.

}
Expand All @@ -263,6 +275,17 @@ func hasModelCostOverlay(costs map[string]any, provider, model string) bool {
return false
}

func hasNonEmptyCostMap(value any) bool {
switch cost := value.(type) {
case map[string]any:
return len(cost) > 0
case map[string]string:
return len(cost) > 0
default:
return false
}
}

func stringSlice(value any) []string {
raw, ok := value.([]any)
if !ok {
Expand Down
108 changes: 63 additions & 45 deletions pkg/cli/model_costs.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package cli
import (
_ "embed"
"encoding/json"
"path"
"strconv"
"strings"
"sync"
Expand Down Expand Up @@ -35,51 +36,44 @@ type modelPriceRecord struct {
pricing map[string]float64
}

var (
modelPriceRecords []modelPriceRecord
modelPricesOnce sync.Once
)
var modelPriceRecords = sync.OnceValue(loadModelPriceRecords)

func initModelPrices() {
modelPricesOnce.Do(func() {
var data modelsCatalogData
if err := json.Unmarshal(modelsJSON, &data); err != nil {
return
}
func loadModelPriceRecords() []modelPriceRecord {
var data modelsCatalogData
if err := json.Unmarshal(modelsJSON, &data); err != nil {
return nil
}

modelPriceRecords = make([]modelPriceRecord, 0)
for providerName, providerData := range data.Providers {
normalizedProvider := strings.ToLower(strings.TrimSpace(providerName))
if normalizedProvider == "" { //nolint:tolowerequalfold
records := make([]modelPriceRecord, 0)
for providerName, providerData := range data.Providers {
normalizedProvider := strings.ToLower(strings.TrimSpace(providerName))
if normalizedProvider == "" { //nolint:tolowerequalfold
continue
}
for modelName, entry := range providerData.Models {
normalizedModel := strings.ToLower(strings.TrimSpace(modelName))
if normalizedModel == "" { //nolint:tolowerequalfold
continue
}
for modelName, entry := range providerData.Models {
normalizedModel := strings.ToLower(strings.TrimSpace(modelName))
if normalizedModel == "" { //nolint:tolowerequalfold
continue
}
normalizedID := normalizedProvider + "/" + normalizedModel
record := modelPriceRecord{
id: normalizedID,
provider: normalizedProvider,
model: normalizedModel,
pricing: make(map[string]float64, len(entry.Cost)),
}
for key, value := range entry.Cost {
if parsed, err := strconv.ParseFloat(strings.TrimSpace(value), 64); err == nil {
record.pricing[key] = parsed
}
record := modelPriceRecord{
id: path.Join(normalizedProvider, normalizedModel),
provider: normalizedProvider,
model: normalizedModel,
pricing: make(map[string]float64, len(entry.Cost)),
}
for key, value := range entry.Cost {
if parsed, err := strconv.ParseFloat(strings.TrimSpace(value), 64); err == nil {
record.pricing[key] = parsed
}
modelPriceRecords = append(modelPriceRecords, record)
}
records = append(records, record)
}
modelCostsLog.Printf("Initialized model price catalog: providers=%d, records=%d", len(data.Providers), len(modelPriceRecords))
})
}
modelCostsLog.Printf("Initialized model price catalog: providers=%d, records=%d", len(data.Providers), len(records))
return records
}

func findModelPricing(provider, model string) (map[string]float64, bool) {
initModelPrices()

normalizedProvider := modelsdev.NormalizeProvider(provider)
normalizedModel := strings.ToLower(strings.TrimSpace(model))
comparableModel := modelsdev.NormalizeComparableModelID(normalizedModel)
Expand All @@ -89,23 +83,21 @@ func findModelPricing(provider, model string) (map[string]float64, bool) {

fullID := normalizedModel
if !strings.Contains(fullID, "/") && normalizedProvider != "" {
fullID = normalizedProvider + "/" + normalizedModel
fullID = path.Join(normalizedProvider, normalizedModel)
}
comparableFullID := modelsdev.NormalizeComparableModelID(fullID)

for _, record := range modelPriceRecords {
if (fullID != "" && record.id == fullID) || (comparableFullID != "" && modelsdev.NormalizeComparableModelID(record.id) == comparableFullID) {
modelCostsLog.Printf("Exact pricing match: provider=%s, model=%s -> %s", provider, model, record.id)
return record.pricing, true
}
if pricing, ok := findExactCatalogPricing(modelPriceRecords(), fullID, comparableFullID); ok {
modelCostsLog.Printf("Exact pricing match: provider=%s, model=%s -> %s", provider, model, fullID)
return pricing, true
}

var bestProviderScoped map[string]float64
bestProviderScopedLen := -1
var bestGeneric map[string]float64
bestGenericLen := -1

for _, record := range modelPriceRecords {
for _, record := range modelPriceRecords() {
comparableRecordModel := modelsdev.NormalizeComparableModelID(record.model)
if record.model == normalizedModel || comparableRecordModel == comparableModel {
if normalizedProvider != "" && record.provider == normalizedProvider {
Expand Down Expand Up @@ -141,15 +133,23 @@ func findModelPricing(provider, model string) (map[string]float64, bool) {
return nil, false
}

func findExactModelPricing(provider, model string) (map[string]float64, bool) {
initModelPrices()
func findExactCatalogPricing(records []modelPriceRecord, fullID, comparableFullID string) (map[string]float64, bool) {
for _, record := range records {
if (fullID != "" && record.id == fullID) || (comparableFullID != "" && modelsdev.NormalizeComparableModelID(record.id) == comparableFullID) {
return record.pricing, true
}
}
return nil, false
}

func findExactModelPricing(provider, model string) (map[string]float64, bool) {
normalizedProvider := modelsdev.NormalizeProvider(provider)
model, _, _ = strings.Cut(strings.TrimSpace(model), "?")
comparableModel := modelsdev.NormalizeComparableModelID(model)

Copilot AI Oct 7, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Handled in b2742c01: query parameters are removed before exact catalog lookup and before workflow overlay injection, with a ?effort=high regression test.

@github-actions github-actions Bot Oct 7, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Query-parameter suffixes are not stripped before the exact-pricing lookup.

configuredModelPricingID strips a ?effort=... suffix from the configured model before calling findExactModelPricing, but the pricing resolver wired up in compile_compiler_setup.go (findExactModelPricing) is also called directly by resolveModelPricingIfMissing via the injected modelPricingResolver, and findExactModelPricing itself does not strip query parameters from model.

Reproduced locally:

findExactModelPricing("openai", "gpt-6.1-sol?effort=high") // => (nil, false)
findExactModelPricing("openai", "gpt-6.1-sol")              // => (pricing, true)

Any workflow configuring model: openai/gpt-6.1-sol?effort=high (a valid Model Alias Format identifier per model_alias_validation.go) will silently lose the catalog pricing match. Strip the query suffix (e.g. strings.Cut(model, "?")) at the top of findExactModelPricing, mirroring what configuredModelPricingID already does. @copilot please address this.

Copilot AI Oct 7, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Handled in b2742c01: the workflow resolver and exact catalog lookup strip query parameters; regression coverage verifies the base model is injected.

if normalizedProvider == "" || comparableModel == "" {
return nil, false
}
for _, record := range modelPriceRecords {
for _, record := range modelPriceRecords() {
if record.provider == normalizedProvider &&
modelsdev.NormalizeComparableModelID(record.model) == comparableModel &&
len(record.pricing) > 0 {
Expand All @@ -159,6 +159,24 @@ func findExactModelPricing(provider, model string) (map[string]float64, bool) {
return nil, false
}

func hasCatalogPricingMatching(provider, pattern string) bool {
normalizedProvider := modelsdev.NormalizeProvider(provider)
comparablePattern := modelsdev.NormalizeComparableModelID(pattern)
if normalizedProvider == "" || comparablePattern == "" {
return false
}
for _, record := range modelPriceRecords() {
if record.provider != normalizedProvider || len(record.pricing) == 0 {
continue
}
matched, err := path.Match(comparablePattern, modelsdev.NormalizeComparableModelID(record.model))
if err == nil && matched {
return true
}
}
return false
}

func usdToAIC(usd float64) float64 {
return usd / 0.01
}
Expand Down
7 changes: 7 additions & 0 deletions pkg/cli/model_costs_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,13 @@ func TestFindModelPricing(t *testing.T) {
assert.InDelta(t, 0.000003, pricing["input"], 1e-12)
}

func TestFindExactModelPricingStripsQueryParameters(t *testing.T) {
t.Parallel()
pricing, ok := findExactModelPricing("openai", "gpt-6.1-sol?effort=high")
require.True(t, ok)
assert.InDelta(t, 0.000002, pricing["input"], 1e-12)
}

func TestFindKimiK3Pricing(t *testing.T) {
t.Parallel()
pricing, ok := findModelPricing("github-copilot", "kimi-k3")
Expand Down
14 changes: 7 additions & 7 deletions pkg/cli/models_command.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,9 +169,9 @@ func refreshObservedArtifacts(ctx context.Context, logsDir string, refreshCount
}

func buildModelCatalogRows() []modelCatalogRow {
initModelPrices()
rows := make([]modelCatalogRow, 0, len(modelPriceRecords))
for _, record := range modelPriceRecords {
records := modelPriceRecords()
rows := make([]modelCatalogRow, 0, len(records))
for _, record := range records {
rows = append(rows, modelCatalogRow{
Provider: record.provider,
Model: record.model,
Expand Down Expand Up @@ -458,12 +458,12 @@ type catalogIndex struct {
}

func makeCatalogIndex() catalogIndex {
initModelPrices()
records := modelPriceRecords()
index := catalogIndex{
fullIDs: make(map[string]struct{}, len(modelPriceRecords)),
bareModels: make(map[string]struct{}, len(modelPriceRecords)),
fullIDs: make(map[string]struct{}, len(records)),
bareModels: make(map[string]struct{}, len(records)),
}
for _, record := range modelPriceRecords {
for _, record := range records {
index.fullIDs[modelsdev.NormalizeComparableModelID(record.id)] = struct{}{}
index.bareModels[modelsdev.NormalizeComparableModelID(record.model)] = struct{}{}
}
Expand Down
Loading
Loading