Repository navigation
Python: fix(lab-tau2): check the over-max warning before the half-of-max warning - #9341
Conversation
…max warning The half-of-max branch ran first and also matched every over-max case, so the red truncation warning was dead code. Swap the branch order. Fixes microsoft#9338 Signed-off-by: Manohar Paturi <186662190+ManoharPaturi@users.noreply.github.com>
GitHub Actions
Active
GitHub Actions
Active
GitHub Actions
Active
agent-framework-automation
Bot
added
python
GitHub Actions
Active
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
MAF Automated Review — Iteration 1
Result: No findings
Scope: full PR (2 commit(s)): 624972ec549c, 0d71d4a447a7
Model: gpt-5.6-sol
Overview
The PR correctly makes the over-budget warning reachable by checking it before the overlapping half-budget condition. The strict truncation threshold and stored-history behavior remain unchanged, while the regression test demonstrates that an over-budget count emits the intended warning. No publishable correctness, compatibility, architectural, or security defect was established.
Reviewed the supplied pull-request change set across correctness, security/reliability, architecture, and failure behavior.
No publishable findings remained after source verification for this scope.
|
/review |
Signed-off-by: Manohar Paturi <186662190+ManoharPaturi@users.noreply.github.com>
GitHub Actions
Active
|
The lab failures were a lint nit on my new test, one long literal. Wrapped and pushed 2faf47a. |
GitHub Actions
Active
Eduard van Valkenburg (eavanvalkenburg)
left a comment
Eduard van Valkenburg (eavanvalkenburg)
left a comment
There was a problem hiding this comment.
Manohar Paturi (@ManoharPaturi) python/packages/lab/tau2/tests/test_sliding_window.py:241 calls logger.remove() without a handler id before the assertion and again in finally. Because this is the real process-global Loguru logger, the test removes every sink for the entire pytest process and leaves later tests with mutated logging state, making the suite order-dependent.
Please avoid removing global handlers. Patch agent_framework_lab_tau2._sliding_window.logger, or retain the id returned by logger.add(...) and remove only that sink in finally.
Signed-off-by: Manohar Paturi <186662190+ManoharPaturi@users.noreply.github.com>
GitHub Actions
Active
Motivation & Context
In
SlidingWindowHistoryProvider._get_token_countthe red "over max tokens, will truncate" warning could never fire. The yellow half-of-max check ran first and also matched every over-max case, so the branch was dead code and the truncation warning was silently lost. Issue #9338 has the repro.Description & Review Guide
Related Issue
Fixes #9338
AI Assistance
Contribution Checklist
breaking changelabel (or add "[BREAKING]" to the title prefix, before or after any language prefix) — a workflow keeps the label and title prefix in sync automatically.