Skip to content

fix(ai): stop shipping a response cap for deepseek-v4-flash - #491

Closed
devops-thiago wants to merge 1 commit into
release/v0.6.0from
fix/deepseek-flash-output-cap
Closed

fix(ai): stop shipping a response cap for deepseek-v4-flash#491
devops-thiago wants to merge 1 commit into
release/v0.6.0from
fix/deepseek-flash-output-cap

Conversation

@devops-thiago

@devops-thiago devops-thiago commented Aug 9, 2026

Copy link
Copy Markdown
Owner

What type of PR is this?

  • 🐛 Bug fix

Description

Fixes a startup regression I introduced in #490.

That PR shipped thrillhousebot.ai.models.deepseek-v4-flash.max-output-tokens=384000 as a default. StartupConfigValidator.validateEffectiveBudget requires output-buffer-tokens >= max-output-tokens for the active model, and the shipped default buffer is 8192. So every deployment naming this model refused to boot — a hard startup failure, not a degraded review:

ThrillhouseBot cannot start — required configuration is missing or invalid:
  - the effective output buffer (16384) must be >= max-output-tokens (384000) for model
    'deepseek-v4-flash' so the token budget reserves the configured response cap

(Reported from a production deployment on a 16384 buffer.)

Kept: max-input-tokens=1000000. That is the part that was genuinely wrong before #490 — the model carried only an empty binding stub, so it fell through to ModelSettings.DEFAULT_MAX_INPUT_TOKENS (128 000) and every review was budgeted at an eighth of the real window.

Reverted: the output cap, back to an empty stub. 384 000 is the model's ceiling, not a sensible per-call request. Beyond the boot failure it has two other costs: it rides the wire as max_tokens on every call, and it reserves 384 K of the token budget away from the diff. A deployment that genuinely wants an explicit cap sets it per-model and raises the buffer to match — which is now spelled out in the properties comment.

Related Issues

Regression from #490. No issue filed — caught in production within the hour.

How Has This Been Tested?

  • Unit tests

The interesting question is why the 2426-test suite went green on a change that cannot boot. validateEffectiveBudget reads activeModel, so the rule only fires for the one model AI_MODEL selects; the shipped table names eighteen. Nothing in the suite activates deepseek-v4-flash, so nothing evaluated the pair.

So the new guard does not test the validator — it walks the shipped table directly and asserts no entry ships a max-output-tokens above its own effective buffer (per-model override, else review.output-buffer-tokens). That holds for any future entry, not just this one.

RednoShippedModelCapCanRefuseToBoot, run against the properties as #490 shipped them:

org.opentest4j.AssertionFailedError: shipped max-output-tokens 384000 for 'deepseek-v4-flash'
exceeds its effective output buffer 8192 — every deployment naming this model would fail to start
==> expected: <true> but was: <false>

That is the production failure reproduced as a test, from the shipped config rather than a mock.

shouldShipDeepSeekV4FlashContextWindowWithoutAResponseCap fails alongside it (it replaces #490's shouldShipDeepSeekV4FlashContextAndOutputCaps, which asserted the value that broke boot).

Green after dropping the cap: Tests run: 8 in that class.

Gates on the final tree:

  • ./mvnw -B spotless:apply / spotless:check — clean
  • ./mvnw -B clean compile spotbugs:checkBugInstance size is 0, BUILD SUCCESS
  • ./mvnw -B clean testTests run: 2427, Failures: 0, Errors: 0, Skipped: 0

Checklist

  • My code follows the project's coding standards
  • I have performed a self-review of my own code
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • I have updated the documentation accordingly
  • My changes generate no new warnings or errors

Additional Notes

Operators already on deepseek-v4-flash and stuck at boot can unblock without waiting for this to merge, by capping to their existing buffer:

THRILLHOUSEBOT_AI_MODELS_DEEPSEEK_V4_FLASH_MAX_OUTPUT_TOKENS=16384

Raising REVIEW_OUTPUT_BUFFER_TOKENS to 384000 also boots, but it carves 384 K out of the input budget — the override above is the better move. After this merges, neither is needed.

The max_tokens-versus-context-window concern I raised on #490 is now moot for the shipped default: with no max-output-tokens, no max_tokens goes out and the provider default applies. It returns only for a deployment that sets the cap deliberately.

Shipping max-output-tokens=384000 as a default refused to boot every
deployment naming the model:

  the effective output buffer (16384) must be >= max-output-tokens (384000)
  for model 'deepseek-v4-flash' so the token budget reserves the configured
  response cap

StartupConfigValidator requires output-buffer-tokens >= max-output-tokens for
the active model, and the shipped default output buffer is 8192. Any operator
switching to this model hit a hard startup failure, not a degraded review.

Keep the 1000000-token input window — that is the part that was actually
wrong before, since the model previously fell back to the 128000 assumption
for unknown models. Drop the output cap back to an empty binding stub: 384000
is the model's ceiling, not a sensible per-call request, and it also rides the
wire as max_tokens on every call. A deployment that wants an explicit cap sets
it per-model and raises the buffer to match.

The suite missed this because the validator rule only fires for the ACTIVE
model, and no test activates deepseek-v4-flash. Add a guard that walks the
shipped table instead, so no future entry can ship a cap above its own
effective buffer.
@github-actions

github-actions Bot commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Scanned Files

None

@thrillhousebot

thrillhousebot Bot commented Aug 9, 2026

Copy link
Copy Markdown

🤖 ThrillhouseBot PR Summary

What this PR does

Reverts the deepseek-v4-flash max-output-tokens default from 384000 to an empty stub so deployments naming the model no longer fail StartupConfigValidator.validateEffectiveBudget when the output buffer is smaller, while keeping the max-input-tokens=1000000 shipped in #490. Adds regression tests: one asserting the model ships no response cap, and one walking the entire shipped model table to ensure no entry's max-output-tokens exceeds its effective output buffer.

Changes Overview

  • Files changed: 2
  • Lines added: +51
  • Lines removed: -9

Changed Files

File Change Summary
src/main/resources/application.properties Modified Clears deepseek-v4-flash max-output-tokens default (384000); keeps 1M input cap; expands comment.
src/test/java/dev/thiagogonzaga/thrillhousebot/config/AiPricingConfigTest.java Modified Replaces output-cap assertion with empty-cap check; adds table-wide no-cap-over-buffer guard.

Risk Assessment

Risk Count
🔴 Critical 0
🟠 High 0
🟡 Medium 0
🔵 Low 0

No new issues found in this PR, but the review cannot be approved until CI is confirmed green.

⚠️ CI Checks Status

Some checks are still pending or have failed:

Check Type Status Detail
test check-run ⏳ Pending -
changes check-run ⏳ Pending -
trivy check-run ⏳ Pending -
actionlint check-run ⏳ Pending -
format check-run ⏳ Pending -
frontend check-run ⏳ Pending -
dependency-review check-run ⏳ Pending -

Automated review by ThrillhouseBot. Reply with /review to re-run.

@thrillhousebot thrillhousebot Bot added bug Something isn't working java Pull requests that update java code testing Test coverage and test quality labels Aug 9, 2026
@codecov

codecov Bot commented Aug 9, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@sonarqubecloud

sonarqubecloud Bot commented Aug 9, 2026

Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working java Pull requests that update java code testing Test coverage and test quality

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant