Skip to content

fix(models): correct cache, usage, streaming, and image outputs - #1653

Merged
Yunnglin merged 1 commit into
modelscope:mainfrom
git-jxj:fix/model-layer-accuracy
Aug 28, 2026
Merged

fix(models): correct cache, usage, streaming, and image outputs#1653
Yunnglin merged 1 commit into
modelscope:mainfrom
git-jxj:fix/model-layer-accuracy

Conversation

@git-jxj

@git-jxj git-jxj commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Summary

This PR fixes six independent model-layer correctness issues:

  • Model memoization could return an instance created for a different eval_type.
  • ModelScope generation with multiple choices only reported usage from one choice.
  • Packed streaming choices could lose their individual finish_reason values.
  • Text-to-image and image-editing outputs omitted the data: URI prefix required by downstream consumers.
  • LiteLLM requests ignored reasoning_history and the configured base URL.
  • Image model outputs stored an epoch timestamp instead of elapsed generation time.

Each behavior is covered by deterministic tests with mocked model or SDK dependencies.

Root Cause

The model cache key did not include eval_type, so otherwise identical task and judge configurations could collide across backends.

ModelScope usage and OpenAI-compatible stream metadata were assigned from a single choice instead of being aggregated or indexed per choice.

Image model adapters serialized raw base64 content and assigned time.time() directly to ModelOutput.time, while downstream APIs expect a data URI and elapsed seconds.

The LiteLLM request builder did not forward the same reasoning-history and base-URL settings used by the OpenAI-compatible path.

Changes

  • Include eval_type in the model memoization key and document the cache contract.
  • Aggregate ModelScope token usage across every generated choice.
  • Track streaming finish_reason by choice index.
  • Emit complete image data URIs from text-to-image and image-editing models.
  • Report elapsed generation time for image outputs.
  • Forward LiteLLM reasoning_history and base_url configuration.
  • Add regression tests for all six behaviors.

Reproduction

On the unmodified base:

same model/config with different eval_type -> the same cached backend instance
ModelScope two-choice usage              -> 10 instead of 20
second packed choice finish_reason       -> stop instead of length
image output consumed as media           -> FileNotFoundError
image ModelOutput.time                   -> approximately 1.7e9 seconds
LiteLLM reasoning/base URL settings      -> silently omitted

The regression suite reports 11 failures against the old source. After this change, each configuration produces the intended backend, metadata, image payload, and timing value.

Validation

Targeted regression tests:

python -m pytest \
  tests/models/test_model_cache.py \
  tests/models/test_modelscope_usage.py \
  tests/models/test_openai_stream_finish_reason.py \
  tests/models/test_image_model_output.py \
  tests/models/test_litellm_reasoning_history.py -q

Result:

13 passed

Model-layer regression suite:

python -m pytest tests/models -q

Result:

62 passed

CI smoke test:

python -m pytest tests/cli/test_all.py::TestRun::test_ci_lite -q -p no:warnings

Result:

1 passed

All current Ruff and repository pre-commit checks passed.

Scope

These changes do not alter prompts, generation parameters, cache lifetime, retry behavior, image encoding content, or non-LiteLLM request routing. The branch is rebased onto the current main and contains one commit.

Include eval_type in model memoization, aggregate ModelScope usage across choices, preserve per-choice stream finish reasons, emit data URIs with elapsed timing for image outputs, and honor LiteLLM reasoning/base URL settings. Add regression coverage for each behavior.
@git-jxj
git-jxj marked this pull request as ready for review August 27, 2026 10:13

@Yunnglin Yunnglin left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM!

@Yunnglin
Yunnglin merged commit 34a3bcf into modelscope:main Aug 28, 2026
3 checks passed
@git-jxj
git-jxj deleted the fix/model-layer-accuracy branch August 29, 2026 14:29
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.

2 participants