Skip to content

fix(api): map persisted combo steps in the playground route simulator - #11882

Closed
NoxzRCW wants to merge 2 commits into
cheaper-inference:release/v3.8.51from
NoxzRCW:fix/simulate-route-combo-steps-11822
Closed

NoxzRCW wants to merge 2 commits into
cheaper-inference:release/v3.8.51from
NoxzRCW:fix/simulate-route-combo-steps-11822

Conversation

@NoxzRCW

@NoxzRCW NoxzRCW commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

Summary

POST /api/playground/simulate-route returned HTTP 500 for every persisted combo, which made the Combo Playground page unusable. Every "Simulate Route" click failed, whichever combo was selected:

{"error":"Simulation error: Cannot read properties of undefined (reading 'length')"}

This is not the "combo not found" path. A bogus comboId correctly returns 404, so the combo is found and then the handler crashes while iterating its targets.

The route read combo.targets, but getCombos() returns records normalized by normalizeComboRecord(), which is schema version 2 and keeps its steps under models as a ComboStep[]. There is no targets property, so comboInfo.targets was always undefined and the loop below dereferenced undefined.length. The inline body.combo branch worked only because the caller supplies targets explicitly per simulateRequestSchema, which is why the dashboard was the only broken path.

There is a second mismatch in the same loop. A step's provider comes from step.providerId, or from the provider/model prefix of step.model, never from a top-level step.provider. So even with targets populated, the connection lookup would have failed to resolve and produced spurious Provider undefined not configured warnings. Both are handled here.

combo-ref and provider-wildcard steps are not expanded by the simulator. Rather than dropping them silently they now surface as warnings, and the wildcard is listed unresolved so the operator can see it is in the route.

Related Issues

Validation

  • Change type: routing
  • Focused tests and category gates from the golden path
  • npm run lint
  • Reconciled with the current active release base; focused checks rerun afterward
  • Production-code changes include a new or updated automated test in this PR
node --import tsx/esm --test tests/unit/playground-simulate-route-combo-steps-11822.test.ts
# tests 6 / pass 6 / fail 0

npm run typecheck:core   # 0 errors
npm run lint             # clean

Tests Added Or Updated

  • tests/unit/playground-simulate-route-combo-steps-11822.test.ts (new)

Covers the three step kinds, providerId winning over the model prefix, a serialized models column, and the malformed inputs (null, unparseable JSON, junk entries) that should degrade to an empty target list rather than throw.

Coverage Notes

The mapping lives in src/lib/combos/simulatorTargets.ts and is fully covered by the new test file. The route itself keeps its existing shape and just calls into it.

Reviewer Notes

I put the mapping in src/lib/combos/ rather than inline in the route, for two reasons. A Next.js App Router route module cannot export anything besides its handlers and the documented route config, so the helper would not have been testable where the issue suggested putting it. And ComboStep already lives under src/lib/combos/, so the mapping sits next to the type it consumes.

Routing itself was never affected. Real requests through these combos always dispatched correctly, this was only the simulator's view of them.

If you would rather have combo-ref steps expanded through resolveComboTargets() instead of surfacing as a warning, say so and I will do that instead. I went with the warning because expanding nested combos in the simulator raises cycle-handling questions that felt out of scope for a 500 fix.

POST /api/playground/simulate-route read combo.targets, but getCombos() returns
records normalized by normalizeComboRecord(), which is schema version 2 and
keeps its steps under models as a ComboStep[]. There is no targets property, so
the target loop dereferenced undefined.length and every persisted combo came
back as HTTP 500. The inline body.combo branch worked only because the caller
supplies targets explicitly, which is why the dashboard was the only broken
path.

A step's provider comes from step.providerId or the provider/model prefix of
step.model, never from a top-level step.provider, so the connection lookup would
have reported "Provider undefined not configured" even once targets existed.
Both are handled here.

combo-ref and provider-wildcard steps are not expanded by the simulator. They
now surface as warnings instead of disappearing.

The mapping lives in src/lib/combos/simulatorTargets.ts rather than in the route
file, since a Next.js route module cannot export anything besides its handlers
and the documented route config.

Closes cheaper-inference#11822
@NoxzRCW
NoxzRCW requested a review from diegosouzapw as a code owner August 28, 2026 11:17
diegosouzapw added a commit that referenced this pull request Aug 28, 2026
…tring-step handling (#11930)

Ports the specific-warning improvement from #11882 (combo-ref/provider-wildcard steps get their own message instead of a generic count) onto #11862's already-merged crash fix. 4/4 focused tests passing.
@diegosouzapw

Copy link
Copy Markdown
Collaborator

Thanks for this — and sorry for the overlap. #11862 landed first and already fixed the
core #11822 crash (reading combo.models instead of the nonexistent combo.targets,
resolving provider via providerId/model-prefix), so this conflicted by the time I got
to it.

Your one genuinely distinct contribution — surfacing combo-ref and provider-wildcard
steps with a specific warning instead of a generic "unsupported step" count, and showing
the wildcard as an unresolved target — was real value worth keeping. I hand-applied that
part onto #11862's code (full credit to your design) as #11930, since your extracted
comboStepsToTargets() helper didn't cover the plain-string step shape #11862 also
handles. Closing this one now that its distinct value has landed. Thank you for the
thorough regression coverage and the clear writeup either way!

muhamadgalihsaputra pushed a commit to niyatna/NiyatnaRoute that referenced this pull request Sep 27, 2026
… keep cheaper-inference#11862's string-step handling (cheaper-inference#11930)

Ports the specific-warning improvement from cheaper-inference#11882 (combo-ref/provider-wildcard steps get their own message instead of a generic count) onto cheaper-inference#11862's already-merged crash fix. 4/4 focused tests passing.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(playground): simulate-route 500s for every persisted combo — reads combo.targets but getCombos() returns models

2 participants