Skip to content

fix(reflect): enforce max_tokens after forced synthesis - #4159

Closed
koriyoshi2041 wants to merge 1 commit into
vectorize-io:mainfrom
koriyoshi2041:rios/fix-reflect-forced-max-tokens
Closed

fix(reflect): enforce max_tokens after forced synthesis#4159
koriyoshi2041 wants to merge 1 commit into
vectorize-io:mainfrom
koriyoshi2041:rios/fix-reflect-forced-max-tokens

Conversation

@koriyoshi2041

Copy link
Copy Markdown
Contributor

Problem

max_tokens is enforced with a post-hoc rewrite when reflect finishes through the done tool, but forced final synthesis returns its answer directly. Long or context-exhausted runs can therefore exceed the caller's visible-length target.

Closes #4156.

Fix

  • Extract the existing final-answer rewrite into a shared helper.
  • Apply it to both done and forced-synthesis completions.
  • Run the rewrite before structured-output extraction so structured output is derived from the bounded answer.
  • Include rewrite usage and trace data in forced-synthesis accounting.

Test

  • uv run --project hindsight-api-slim pytest -q hindsight-api-slim/tests/test_reflect_agent.py -k 'not TestDirectiveLeakageOnEmptyBank and not TestContextOverflowIntegration and not TestMentalModelShortCircuitRealLLM' (55 passed)
  • uv run --project hindsight-api-slim ruff check hindsight-api-slim/hindsight_api/engine/reflect/agent.py hindsight-api-slim/tests/test_reflect_agent.py
  • uv run --project hindsight-api-slim ruff format --check hindsight-api-slim/hindsight_api/engine/reflect/agent.py hindsight-api-slim/tests/test_reflect_agent.py
  • git diff --check

The four excluded tests require an embedded PostgreSQL extra or live LLM credentials; the first full-file run reached 55 passing unit tests before those environment-only setup errors.

Risk

Forced synthesis may make one additional LLM call when its answer exceeds max_tokens. Calls without max_tokens and answers already within budget are unchanged, and the rewrite remains governed by reflect_max_completion_tokens rather than using the visible target as a hard transport cap.

@koriyoshi2041

Copy link
Copy Markdown
Contributor Author

Closing as superseded by #4180, which covers the same forced-synthesis length boundary and landed the stronger shared rewrite path. Thanks for picking this up.

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.

reflect: forced final synthesis lost max_tokens enforcement in #3389 (rewrite only runs on the done path)

1 participant