Skip to content

[OMEGA-350] Add unit tests for LLM token-budget handling - #350

Open
TossSky wants to merge 1 commit into
OMEGA-350-openrouter-reasoning-takes-the-whole-token-budget-glm-comes-back-emptyfrom
OMEGA-350-retry-without-reasoning
Open

TossSky wants to merge 1 commit into
OMEGA-350-openrouter-reasoning-takes-the-whole-token-budget-glm-comes-back-emptyfrom
OMEGA-350-retry-without-reasoning

Conversation

@TossSky

@TossSky TossSky commented Sep 14, 2026

Copy link
Copy Markdown
Collaborator

Description

Follow-up to #337, based on its branch. Unit tests only, no behaviour change.

Autotests/unit/test_llm_budget.py (15 tests), registered in run_mandatory, covers what #337 added: the finish_reason and incomplete_reason checks before the notice, the notice itself on an empty reply that ran out of budget for OpenRouter, OpenAI and ASI:One, the ASI:One reasoning budget mapping, the OpenRouter reasoning body, [LLM_USAGE] at INFO on both APIs, and an API error returning an empty string. The provider modules are loaded by file path with openai and config stubbed, so the tests need no container, network or token.

Earlier versions of this PR also changed provider behaviour: a retry without reasoning, one notice per streak, and keeping a truncated reply out of the loop. All three are dropped after the discussion here and in #337.

How Has This Been Tested?

CI: tests/pytest.sh 65 passed, Phase 1 144 passed (129 + 15 new), Phase 2 5 passed and 1 skipped, MeTTa tests green.

Checklist

  • PR contains autogenerated code
  • Self-review completed
  • Test scenarios above are passed with the version of the code from PR

@paul-v-snet

paul-v-snet commented Sep 16, 2026

Copy link
Copy Markdown
Collaborator

@TossSky,

When the token budget runs out (finish_reason=length, or status=incomplete with max_output_tokens on the Responses API), OpenRouter and OpenAI retry the request once without reasoning: reasoning: {"enabled": false} and effort: "none". Providers without such a switch don't retry.

I disagree. This could lead to hidden additional costs. I believe that sending a message to the chat is enough in this case. We should not make additional hidden requests here.

I also discovered several critical issues in these changes:

  1. You're retrying with extra_body["reasoning"] = {"enabled": False}, but, for example, GLM-5.3 and newer models do not support disabling reasoning, so this will lead to an error response. See: https://docs.z.ai/guides/capabilities/thinking#core-parameters.
    This may also apply to other models.
  2. You don't check whether reasoningMode is already configured as none (disabled), so even if it is already disabled, you retry anyway.
  3. In any case, this may silently lead to worse LLM accuracy, and the user will be forced to guess why.

If the retry runs out too, the user gets the explanation once per streak. Further failures return "" until a normal reply comes back, so history no longer fills up with copies.

And what happens if a normal reply never comes back? The agent will still look like it's completely ignoring the user.

A reply cut off at the token limit is never passed to the loop. The user gets a short "answer was cut off" message instead.

Why? The user paid for this response, but we just discard it. I believe a better approach here is to simply append "The agent's answer was cut off..." to the LLM response.
If I missed something please explain the reason in more detail.


In addition, as I see it, the refactoring was mostly caused by the retry implementation. If we decide that it's not necessary, then most of the refactoring is not necessary either, so we could leave only:

  • Unit tests
  • Adding a message to truncated LLM responses
  • INFO log level for LLM usage

These changes could be added directly to #337 after I apply the changes described below, and this PR could then be closed.


What I should update in #337 in that case:

  • Fix the hardcoded "thinking_budget": 6000 and apply mapping similar to what we do for Anthropic.

cc: @vsbogd, please take a look at both PRs and share your opinion when you have some free time.

@vsbogd

vsbogd commented Sep 17, 2026

Copy link
Copy Markdown
Member

When the token budget runs out (finish_reason=length, or status=incomplete with max_output_tokens on the Responses API), OpenRouter and OpenAI retry the request once without reasoning: reasoning: {"enabled": false} and effort: "none".

We discussed this before and agreed that automatic retry is not needed. My suggestion was not do this because model without reasoning has poor performance. But at the moment I think I didn't take into account different kinds of LLM requests we have.

We have reactive agent iterations when agent responses to some request from the user. In such situation notifying user that reasoning is failed because of budget it correct. We have proactive agent iterations when agent pursues its own goals or trying to help user with previous tasks. For this mode it may look surprisingly to user when agent reports about budget exhausted without any input from user. In the last case it sounds more logical to notify LLM about results of the previous iteration. And retrying request automatically without reasoning makes more sense.

  1. From the Omega architecture perspective it looks like retry logic should be part of the main loop rather then be part of the LLM provider implementation. In case it is implemented in the main loop it will be reused by different providers for free. Thus I would suggest skip the retry logic change suggested by this PR.

  2. I would suggest to discuss retry logic further, try to write some testing scenarios (or use-cases) and create clear requirements for retry logic. Also I think retry logic implementation is better to be done after OmegaV2 ([OMEGA-374] Omega V2: using tools API fields to pass the list of tools to LLM #349) is merged as [OMEGA-374] Omega V2: using tools API fields to pass the list of tools to LLM #349 potentially conflicts with retry logic implementation as well.

  3. I think [OMEGA-350] reasoning takes the whole token budget #337 can be merged as is if it passes all tests except retry logic scenario. [OMEGA-350] reasoning takes the whole token budget #337 fixes specific budget issue and I think it is good in this sense.

@TossSky
TossSky force-pushed the OMEGA-350-retry-without-reasoning branch from 054dda0 to 783ef67 Compare September 17, 2026 14:34
@TossSky

TossSky commented Sep 17, 2026

Copy link
Copy Markdown
Collaborator Author

@paul-v-snet @vsbogd the retry is dropped and the branch is rebased onto f3dd502. What is left here: the notice is sent once per streak, a truncated reply is not run, and the unit tests.

On the retry, agreed, and two of Paul's points were right: GLM-5.3 and newer cannot disable reasoning, and I did not check for reasoningMode already set to none. A third one came up while I was checking them: if the retry itself failed, my code returned "", so the user would have got no message at all. Moving it to the main loop after #349 sounds right, and I can write the scenarios for it when we get there.

it may look surprisingly to user when agent reports about budget exhausted without any input from user

That is what the once-per-streak change is for. On f3dd502 at maxOutputToken: 120 the notice is built on every empty iteration: 54 of them landed in history in one run, 45 copies ended up inside a single prompt, and the user's first request was gone from all of the last 10 prompts. The same scenario on this branch gives 15 notices in history, at most 16 copies in one prompt, and the first request present in all of the last 10. The user sees the same thing either way, because send drops a message equal to the last one sent (channels.metta:26): in the 120 run on f3dd502 the 87 notices produced exactly one IRC message.

I believe a better approach here is to simply append "The agent's answer was cut off..." to the LLM response

In V1 the loop parses the reply as text, so a truncated command runs before anyone reads the note. In the 120 run a cut-off write-file /tmp/paths.txt followed by "that didn't include" was executed and wrote that fragment to the file (WRITE-VERIFIED ... bytes=28). Appending a hint does not prevent that. In #354 the hint works, because a truncated tool call fails to parse and never runs.

Tested on the rebased branch: CI 65 / 149 / 5+1, plus live runs on OpenRouter with z-ai/glm-5.2 at the default budget and at 120, OpenAI gpt-5.5 at 120, and Anthropic at the default. Each of the 5 cut-off replies in the 120 run was replaced by the notice and none of them ran.

Neither change depends on the retry, so they can land here or go straight into #337, whichever you prefer.

@TossSky
TossSky force-pushed the OMEGA-350-retry-without-reasoning branch from 783ef67 to 49375ed Compare September 17, 2026 16:30
@TossSky TossSky changed the title [OMEGA-350] Retry without reasoning and report token-limit replies once [OMEGA-350] Add unit tests for LLM token-budget handling Sep 17, 2026
@TossSky

TossSky commented Sep 17, 2026

Copy link
Copy Markdown
Collaborator Author

@paul-v-snet @vsbogd trimmed this PR to the tests. The retry, the once-per-streak notice and the truncated-reply guard are all out.

On the notice in history you are right, and the screenshot settles it: the agent reads its own error and splits the answer by itself, and my flag would have taken that away. My numbers for the flooded prompt came from maxOutputToken: 120, and at the default budget there is nothing to fix there, 3 to 4 out-of-budget replies per 76 to 95 calls and at most 4 copies in a prompt. A lower bound on maxOutputToken sounds right to me.

The truncated reply is still worth fixing, in one run a cut-off write-file ran and put a fragment of the model's sentence into the file. Your shape is better than mine, so I will take it here once you add it.

What is left in this PR covers your own changes: the finish_reason and incomplete_reason checks, the ASI:One budget and the OpenRouter reasoning body.

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.

3 participants