test: TestOllamaFactoryMaxTokens references removed `max_tokens` field (3 pre-existing failures)
Nobody has claimed this yet.
Assessment
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Newbie friendliness
- 68/100
Research direction
Start with tests/unit/test_ollama_embedding_factory.py::TestOllamaFactoryMaxTokens and inspect EmbeddingModelConfig at embedding_config.py:97. Run the provided pytest command, then align the three outdated tests with the current schema and verify that the targeted test class no longer reports the three construction failures.
Written by the indexing model from the issue text.
Description
Problem
tests/unit/test_ollama_embedding_factory.py::TestOllamaFactoryMaxTokens has 3 tests that construct EmbeddingModelConfig(..., max_tokens=...). However, EmbeddingModelConfig no longer has a max_tokens field (the model sets extra="forbid" at embedding_config.py:97), so all 3 fail at construction time with:
pydantic_core._pydantic_core.ValidationError: 1 validation error for EmbeddingModelConfig
max_tokens
Extra inputs are not permitted
Affected tests
TestOllamaFactoryMaxTokens::test_custom_max_tokens_is_forwardedTestOllamaFactoryMaxTokens::test_none_max_tokens_uses_defaultTestOllamaFactoryMaxTokens::test_openai_factory_max_tokens_also_forwarded
Reproduction
./.venv/bin/python -m pytest -o addopts="" tests/unit/test_ollama_embedding_factory.py::TestOllamaFactoryMaxTokens -v
Root cause
The max_tokens field was removed from EmbeddingModelConfig in an earlier commit, but these tests were not updated. They appear to predate the schema change.
Impact
- Fails on
mainbaseline (unrelated to any recent PR). - Any CI run touching
test_llama_embedding_factory.pyshows 3 red, which can mask or be mistaken for real regressions introduced by other PRs.
Suggested fix
Either delete the TestOllamaFactoryMaxTokens class (if max_tokens is no longer a supported concept) or update the assertions to reflect the current schema (e.g., assert the field is rejected, or test whatever replaced it).
Context
Spotted while working on PR #2317 (ollama _provider attribution fix), which also modifies test_ollama_embedding_factory.py. These 3 failures are independent of that PR — confirmed via git stash comparison (same 3 fail on the base commit).
- Dominant language
- Python
- Stars
- 38.1k
- Forks
- 3k
- Avg merge
- 1d 10h
- Merged PRs (30d)
- 418
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
More from volcengine/OpenViking
-
Qwen multiline provenance output causes rewrite=failed; issue also exists in current master/main Open
Difficulty 2/5 1-3 hours Newbie friendliness 76/100
volcengine/OpenViking#5011 · 1 comment ·
-
enhancement
Difficulty 2/5 1-3 hours Newbie friendliness 72/100
volcengine/OpenViking#4117 · 1 comment ·
-
bug
Difficulty 2/5 1-3 hours Newbie friendliness 78/100
volcengine/OpenViking#3709 ·
-
bug
Difficulty 2/5 1-3 hours Newbie friendliness 76/100
volcengine/OpenViking#3508 · 8 comments ·
-
Difficulty 2/5 1-3 hours Newbie friendliness 76/100
volcengine/OpenViking#3171 ·
All issues in volcengine/OpenViking
Similar issues
-
Difficulty 2/5 1-3 hours Newbie friendliness 88/100
-
Difficulty 2/5 1-3 hours Newbie friendliness 82/100
-
Difficulty 2/5 1-3 hours Newbie friendliness 78/100
-
enhancement
Difficulty 2/5 1-3 hours Newbie friendliness 72/100
-
Difficulty 2/5 1-3 hours Newbie friendliness 74/100