-
Notifications
You must be signed in to change notification settings - Fork 628
fix(catalog): synthesize incomplete combo members with context fallback #1163
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.
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
base: dev
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -607,6 +607,104 @@ export function applyConfigHintsToCachedModels(name: string, prov: OcxProviderCo | |
| return models.map(model => applyProviderConfigHints(name, prov, model, contextCap)); | ||
| } | ||
|
|
||
|
|
||
| /** | ||
| * Last-resort context window for combo member synthesis when discovery and | ||
| * provider config both omit one. Matches the catalog entry default in | ||
| * `normalizeRoutedCatalogEntry` so incomplete live rows still catalog. | ||
| */ | ||
| const COMBO_MEMBER_CONTEXT_FALLBACK = 128_000; | ||
|
|
||
| /** | ||
| * Resolve a combo target to a catalog member for derivation. | ||
| * Prefer discovery metadata; when the target is missing from the gather map or | ||
| * lacks a positive contextWindow, synthesize from the (registry-enriched) | ||
| * provider config so combos remain catalogued when targets are configured but | ||
| * discovery metadata is incomplete. Disabled providers stay unresolved. | ||
| * When hints still omit contextWindow, prefer known maxInputTokens, else | ||
| * COMBO_MEMBER_CONTEXT_FALLBACK so a live row without ctx does not drop the | ||
| * whole combo from the public catalog. | ||
| */ | ||
| export function resolveComboCatalogMember( | ||
| target: { provider: string; model: string }, | ||
| memberByKey: ReadonlyMap<string, CatalogModel>, | ||
| providers: ReadonlyMap<string, OcxProviderConfig>, | ||
| contextCap?: number, | ||
| ): CatalogModel | undefined { | ||
| const existing = memberByKey.get(targetKey(target)); | ||
| const prov = providers.get(target.provider); | ||
| // Disabled providers never contribute members — even a complete discovery row | ||
| // is unusable for catalog derivation while the provider is off. | ||
| if (prov?.disabled === true) return undefined; | ||
|
|
||
| // Complete live/configured rows still honor providerContextCaps so a high | ||
| // discovery window cannot outrun an operator-configured cap. | ||
| if ( | ||
| existing | ||
| && typeof existing.contextWindow === "number" | ||
| && existing.contextWindow > 0 | ||
| ) { | ||
| const capped = applyProviderContextCap(existing.contextWindow, contextCap); | ||
| if (capped === undefined || capped === existing.contextWindow) return existing; | ||
|
Comment on lines
+647
to
+648
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When a combo target is the canonical Useful? React with 👍 / 👎. |
||
| const maxInput = typeof existing.maxInputTokens === "number" && existing.maxInputTokens > 0 | ||
| ? Math.min(existing.maxInputTokens, capped) | ||
| : capped; | ||
| return { | ||
| ...existing, | ||
| contextWindow: capped, | ||
| maxInputTokens: maxInput, | ||
| contextCap, | ||
| contextCapped: true as const, | ||
| }; | ||
| } | ||
|
|
||
| const base: CatalogModel = existing ?? { | ||
| id: target.model, | ||
| provider: target.provider, | ||
| }; | ||
| const hinted = prov | ||
| ? applyProviderConfigHints(target.provider, prov, base, contextCap) | ||
| : base; | ||
| const hintedContext = typeof hinted.contextWindow === "number" && hinted.contextWindow > 0 | ||
| ? hinted.contextWindow | ||
| : undefined; | ||
| // Prefer a known positive maxInputTokens over inventing 128k when discovery | ||
| // advertised an input limit but no context window (common thin /models rows). | ||
| const knownMaxInput = typeof hinted.maxInputTokens === "number" && hinted.maxInputTokens > 0 | ||
| ? hinted.maxInputTokens | ||
| : (typeof base.maxInputTokens === "number" && base.maxInputTokens > 0 | ||
| ? base.maxInputTokens | ||
| : undefined); | ||
| const uncappedContext = hintedContext | ||
| ?? knownMaxInput | ||
| ?? (existing || prov ? COMBO_MEMBER_CONTEXT_FALLBACK : undefined); | ||
|
Comment on lines
+678
to
+680
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When a combo target is a row the catalog intentionally filtered out, such as AGENTS.md reference: src/AGENTS.md:L15-L18 Useful? React with 👍 / 👎. |
||
| if (uncappedContext === undefined) return undefined; | ||
| const usedFallback = hintedContext === undefined; | ||
| const cappedContext = applyProviderContextCap(uncappedContext, contextCap); | ||
| const contextWindow = cappedContext ?? uncappedContext; | ||
| const fallbackCapped = usedFallback | ||
| && contextCap !== undefined | ||
| && cappedContext !== undefined | ||
| && cappedContext !== uncappedContext; | ||
|
|
||
| const inputModalities = hinted.inputModalities ?? base.inputModalities ?? ["text"]; | ||
| const reasoningEfforts = hinted.reasoningEfforts | ||
| ?? (prov ? configuredReasoningEfforts(prov, target.model) : undefined) | ||
| ?? base.reasoningEfforts; | ||
| const maxInputTokens = knownMaxInput !== undefined | ||
| ? Math.min(knownMaxInput, contextWindow) | ||
| : contextWindow; | ||
|
|
||
| return { | ||
| ...hinted, | ||
| inputModalities, | ||
| ...(reasoningEfforts !== undefined ? { reasoningEfforts } : {}), | ||
| contextWindow, | ||
| maxInputTokens, | ||
| ...(fallbackCapped ? { contextCap, contextCapped: true as const } : {}), | ||
| }; | ||
| } | ||
|
|
||
| export function isDatedVariantId(liveId: string, configuredId: string): boolean { | ||
| if (!liveId.startsWith(`${configuredId}-`)) return false; | ||
| return /^\d{8}$/.test(liveId.slice(configuredId.length + 1)); | ||
|
|
@@ -1273,21 +1371,26 @@ async function gatherRoutedModelsUncached( | |
| if (!memberByKey.has(key)) memberByKey.set(key, synthetic); | ||
| } | ||
| } | ||
| // Enriched (registry-hydrated) provider clones — shared by combo member synthesis and | ||
| // custom-model vision-sidecar inheritance so both see the same merged registry view. | ||
| const enrichedByName = new Map(activeProviders.map(provider => [provider.name, provider.provider])); | ||
| for (const id of listComboIds(config)) { | ||
| const combo = getCombo(config, id); | ||
| if (!combo) continue; | ||
| const members = combo.targets | ||
| .map(target => memberByKey.get(targetKey(target))) | ||
| .map(target => resolveComboCatalogMember( | ||
| target, | ||
| memberByKey, | ||
| enrichedByName, | ||
| providerContextCap(config, target.provider), | ||
|
Comment on lines
+1381
to
+1385
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When a combo target is only registered through Useful? React with 👍 / 👎. |
||
| )) | ||
|
Comment on lines
+1374
to
+1386
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Description: Verify the disabled-provider signal reaching resolveComboCatalogMember.
set -euo pipefail
echo "=== disabledNativeSlugs definition (does it consider providers.openai.disabled?) ==="
ast-grep run --pattern 'export function disabledNativeSlugs($$$) { $$$ }' --lang typescript src
echo
echo "=== all call sites of resolveComboCatalogMember (which map is passed?) ==="
rg -nP -C 8 '\bresolveComboCatalogMember\s*\(' --type=ts
echo
echo "=== construction of capture.providers (confirm the disabled filter) ==="
rg -nP -C 4 'disabled\s*!==\s*true' --type=ts -g 'src/**'
echo
echo "=== any integration test that disables a provider AND configures a combo ==="
rg -nP -C 10 'disabled:\s*true' --type=ts -g 'tests/**' | rg -n -C 10 'combos'Repository: lidge-jun/opencodex Length of output: 242 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "=== files containing resolveComboCatalogMember/disabledNativeSlugs ==="
rg -n 'resolveComboCatalogMember|disabledNativeSlugs|disabledNative|activeProviders|capture\.providers|disabledNativeSlugs' --type=ts -g 'src/**' -g 'tests/**'
echo
echo "=== provider-fetch relevant sections ==="
python3 - <<'PY'
from pathlib import Path
p = Path('src/codex/catalog/provider-fetch.ts')
lines = p.read_text().splitlines()
for start,end in [(1320,1405),(600,695)]:
print(f"\n--- {p}:{start}-{end} ---")
for i in range(start,end+1):
if i <= len(lines):
print(f"{i:5}: {lines[i-1]}")
PY
echo
echo "=== tests likely relevant ==="
rg -n -C 12 'disabled:\s*true|openai|combos|resolveComboCatalogMember|catalog' tests --type=ts -g 'tests/**' | head -n 300Repository: lidge-jun/opencodex Length of output: 354 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "=== locate provider-fetch file ==="
fd -a 'provider-fetch\.ts$' . || true
echo
echo "=== symbols across tracked TypeScript files ==="
git ls-files '*.ts' | xargs rg -n 'resolveComboCatalogMember|disabledNativeSlugs|activeProviders|capture\.providers|disabled\s*!==\s*true' || true
echo
echo "=== locate test files ==="
git ls-files 'tests/**/*.ts' | xargs rg -n -C 10 'disabled:\s*true|combos|catalog|resolveComboCatalogMember|enrichedByName|capture\.providers' || true
echo
echo "=== provider-fetch key ranges if present ==="
if [ -f src/codex/catalog/provider-fetch.ts ]; then
python3 - <<'PY'
from pathlib import Path
lines = Path('src/codex/catalog/provider-fetch.ts').read_text().splitlines()
for range in [(620,690),(1340,1400),(400,425)]:
print(f"\n--- ranges {range} ---")
for i in range(*range):
if i <= len(lines):
print(f"{i:5}: {lines[i-1]}")
PY
fiRepository: lidge-jun/opencodex Length of output: 17252 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "=== provider-fetch key ranges ==="
python3 - <<'PY'
from pathlib import Path
p = Path('src/codex/catalog/provider-fetch.ts')
lines = p.read_text().splitlines()
ranges = [(620,690),(1280,1415),(395,425)]
for start,end in ranges:
print(f"\n--- {p}:{start}-{end} ---")
for i in range(start, min(end, len(lines)) + 1):
print(f"{i:5}: {lines[i-1]}")
PY
echo
echo "=== metadata definitions ==="
python3 - <<'PY'
from pathlib import Path
p = Path('src/codex/catalog/metadata.ts')
lines = p.read_text().splitlines()
ranges = [(90,170)]
for start,end in ranges:
print(f"\n--- {p}:{start}-{end} ---")
for i in range(start, min(end, len(lines)) + 1):
print(f"{i:5}: {lines[i-1]}")
PY
echo
echo "=== unit tests around resolveComboCatalogMember ==="
python3 - <<'PY'
from pathlib import Path
p = Path('tests/codex-catalog.test.ts')
lines = p.read_text().splitlines()
ranges = [(740,835)]
for start,end in ranges:
print(f"\n--- {p}:{start}-{end} ---")
for i in range(start, min(end, len(lines)) + 1):
print(f"{i:5}: {lines[i-1]}")
PY
echo
echo "=== native-model-toggle tests ==="
python3 - <<'PY'
from pathlib import Path
p = Path('tests/native-model-toggle.test.ts')
lines = p.read_text().splitlines()
for i, line in enumerate(lines, 1):
if 'disabledNativeSlugs' in line:
print(f"{i}: {line}")
PY
cat -n tests/native-model-toggle.test.ts | sed -n '1,90p'
echo
echo "=== exact resolveComboCatalogMember call sites ==="
rg -n -C 12 '\bresolveComboCatalogMember\s*\(' --type=ts -g 'src/codex/catalog/provider-fetch.ts' || true
echo
echo "=== exact activeProviders usage around catalog build ==="
rg -n -C 8 '\bactiveProviders\b' --type=ts -g 'src/codex/catalog/provider-fetch.ts' || true
echo
echo "=== deterministic behavior probe from source extraction ==="
python3 - <<'PY'
import re
from pathlib import Path
src = Path('src/codex/catalog/provider-fetch.ts')
txt = src.read_text()
m = re.search(r'buildCatalogEntries[\s\S]*?const enrichedByName = new Map\([\s\S]*?\);\s*for \(const id of listComboIds\([\s\S]*?resolveComboCatalogMember\([\s\S]*?\)\)\s*\]', txt)
print("buildCatalogEntries block extracted:", bool(m))
if m:
code = m.group(0)
print("includes activeProviders.map provider.disabled filter:", ".filter(([, provider]) => provider.disabled !== true)" in code or "provider.disabled !== true" in code.split("const activeProviders = capture.providers;")[-1].split("const enrichedByName")[0])
print("enrichedByName source:", [x.strip() for x in re.findall(r'const enrichedByName = new Map\([^;]+;\n', code)])
print("comboProviderView construction mentioned:", "comboProviderView" in code or "Object.entries(config.providers)" in code)
print("native injection call present:", "disabledNativeSlugs(config)" in code)
metadata = Path('src/codex/catalog/metadata.ts').read_text()
m2 = re.search(r'export function disabledNativeSlugs\([\s\S]*?\n\}', metadata)
print("\ndisabledNativeSlugs definition:")
print(m2.group(0) if m2 else "not found")
PYRepository: lidge-jun/opencodex Length of output: 27361 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "=== doc references ==="
rg -n -C 4 'disabled provider|disabled providers|catolog|catalog' docs-site/src/content/docs/reference/configuration/routing.md docs-site/src/content/docs/zh-cn/reference/configuration/routing.md 2>/dev/null || true
echo
echo "=== combo disable/route checks around providers.disabled ==="
rg -n -C 6 'target\.provider.*disabled|disabled.*target\.provider|combos.*disabled|disabled.*combos|disabledModels' src --type=ts -g 'src/combos/**' -g 'src/codex/**' -g 'src/router.ts' || true
echo
echo "=== unit tests: provider disabled with combos/native slugs ==="
python3 - <<'PY'
from pathlib import Path
text = Path('tests/codex-catalog.test.ts').read_text()
print('has disabled=true in provider map:', 'disabled: true' in text)
print('has openai provider in tests:', 'openai' in Path('tests/codex-catalog.test.ts').read_text())
print('has combos in tests:', 'combos' in text)
PYRepository: lidge-jun/opencodex Length of output: 25622 Pass a combo provider view that keeps disabled providers visible.
Fix the combo-member lookup by passing the full configured provider map, preferring enriched clones where present, without changing the custom-model inheritance that needs 🧰 Tools🪛 ast-grep (0.45.0)[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec. (detect-child-process-typescript) 🤖 Prompt for AI AgentsSource: Path instructions |
||
| .filter((member): member is CatalogModel => member !== undefined); | ||
| const derived = deriveComboCatalogModel(id, combo, members); | ||
| if (derived) all.push(derived); | ||
| else warnUncataloguedComboOnce(id, combo, members, localOmissions); | ||
| } | ||
| replaceLastComboCatalogOmissions(localOmissions); | ||
| all.sort((a, b) => (a.provider === b.provider ? a.id.localeCompare(b.id) : a.provider.localeCompare(b.provider))); | ||
| // Enriched (registry-hydrated) provider clones, keyed by name — the same view used above so | ||
| // custom rows get the same noVisionModels / inputModalities treatment as discovered rows. | ||
| const enrichedByName = new Map(activeProviders.map(provider => [provider.name, provider.provider])); | ||
| // Provider-derived rows keyed by their Codex-facing slug: a custom override replaces the row | ||
| // with the same slug below, so that row's provider capability metadata is the inheritance source. | ||
| const replacedByRoutedSlug = new Map(all.map(model => [routedSlug(model.provider, model.id), model])); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This updates only the English and zh-cn routing references with the new combo catalog fallback, but the ru, ja, and ko versions of the same section still say combos are omitted when context metadata is absent (for example
docs-site/src/content/docs/ru/reference/configuration/routing.md:93-100,.../ja/...:82-86, and.../ko/...:81-84). That leaves non-English users with the old eligibility rule and contradicts this new documented behavior, so the remaining locale pages should be updated or made non-contradictory.AGENTS.md reference: docs-site/AGENTS.md:L7-L10
Useful? React with 👍 / 👎.