Skip to content

fix(agent): reject stale 32k metadata for MiniMax - #24436

Closed
luoxiao6645 wants to merge 1 commit into
NousResearch:mainfrom
luoxiao6645:fix/24140-context-floor-regression-clean
Closed

fix(agent): reject stale 32k metadata for MiniMax#24436
luoxiao6645 wants to merge 1 commit into
NousResearch:mainfrom
luoxiao6645:fix/24140-context-floor-regression-clean

Conversation

@luoxiao6645

@luoxiao6645 luoxiao6645 commented May 12, 2026

Copy link
Copy Markdown

related #24140

Summary

  • reject stale 32K context metadata for MiniMax models the same way we already do for Kimi
  • invalidate cached 32K MiniMax entries so previously working Telegram and cron setups recover after upgrade
  • add regression tests covering MiniMax cache invalidation, OpenRouter fallback rejection, and non-MiniMax safety

Root Cause

get_model_context_length() already had a targeted guard for Kimi models that were wrongly underreported as 32768 by stale third-party metadata. MiniMax models still lacked the same protection.

That left two failure paths for #24140:

  • a stale persisted 32768 cache entry for MiniMax-M2.7 or related models would win before any provider fallback
  • unknown-provider fallback could still accept OpenRouter metadata reporting MiniMax at 32768

Either path tripped Hermes' existing 64K minimum-context guard and caused Telegram / cron failures even though MiniMax's real context window is 204800.

Testing

  • .\venv\Scripts\python -m pytest tests/agent/test_model_metadata.py tests/agent/test_minimax_provider.py

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint provider/minimax MiniMax (Anthropic transport) labels May 12, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for isolating the stale-MiniMax metadata path. The core premise remains present on current main: agent/model_metadata.py:2125-2178 returns a cached MiniMax 32K value, while the MiniMax fallback remains 204,800 at agent/model_metadata.py:281-287.

Problems

  • Current main added a separate explicit-OpenRouter path after this PR. At agent/model_metadata.py:2357-2367, a provider="openrouter" MiniMax entry with context_length=32768 is still accepted because the guard is Kimi-only. The submitted diff changes the generic fallback but cannot cover this later-added sibling path unchanged.

Suggested changes

  • During salvage, use the shared predicate in the current cache, Nous fallback, explicit OpenRouter, and unknown-provider OpenRouter paths (agent/model_metadata.py:1978-1989, 2125-2145, 2357-2367, 2390-2402).
  • Add an explicit provider="openrouter" MiniMax 32K regression case alongside the submitted provider-empty fallback test.

This is an automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-caching Sweeper risk: may break/degrade prompt caching or cache-key stability (invariant) sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 13, 2026

@GottZ GottZ left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was generated by AI during triage.

Summary

Two PRs address the same root cause: stale 32K cache/OpenRouter metadata is accepted for MiniMax models whose actual fallback context is 204,800 tokens. Both diffs add MiniMax detection, unify the existing Kimi/MiniMax stale-underreport predicate, apply it to three resolution paths, and add equivalent regression coverage.

Related pull requests

  • #24436 related — (+69/-9) — keep open, but update before merge: the diff invalidates stale MiniMax cache entries and rejects 32K metadata in the Nous and generic unknown-provider OpenRouter paths while preserving unrelated 32K values. As identified by the contributor keep_open review, current main now has an additional explicit provider="openrouter" path that this diff does not guard; salvage should apply the shared predicate there and add the requested regression test.
  • #24552 [closed] duplicate — (+73/-8) — duplicate, closed in favor of #24436: its diff implements essentially the same shared MiniMax/Kimi predicate, three guard-site changes, and regression tests, so it remains relevant as the rebased comparison that previously resolved an _safe_ctx conflict. It does not provide a distinct fix to retain separately and likewise predates the explicit-OpenRouter-path requirement identified on #24436.

Duplicates

#24436 and #24552 implement essentially the same stale-32K MiniMax metadata fix; #24552 was closed in favor of #24436.

Suggested consolidation

Merge #24436 after rebasing it onto current main, extending the shared stale-underreport predicate to the explicit provider="openrouter" path, and adding an explicit-provider MiniMax 32K regression test, as required by the contributor keep_open review. Keep #24552 closed as the duplicate superseded by the consolidated #24436 implementation.

Complex graph

flowchart LR
    classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
    classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
    classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
    classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
    classDef best stroke-width:3px,stroke:#b45309
    classDef target stroke-width:3px,stroke:#4338ca
    subgraph Dup24436 ["PRs duplicating each other"]
        P24436["PR #24436 (open)"]
        P24552["PR #24552 (closed)"]
    end
    class P24436 open
    class P24552 closed
    class P24436 target
    click P24436 "https://github.com/NousResearch/hermes-agent/pull/24436"
    click P24552 "https://github.com/NousResearch/hermes-agent/pull/24552"
Loading

Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed or no verify verdict yet (state tag in the node label).

Cross-PR triage: Reviewed 2 pull requests and 0 issues in this complex. Each diff was read against this issue; Assessment working set: 14 kB of PR diffs, 3 kB of issue/PR text, 3 kB of discussion (4 comments), 1 verify verdict. verdicts reflect diff content, not PR titles. Part of an automated triage batch.

@teknium1

Copy link
Copy Markdown
Contributor

Merged via PR #85506 (merge commit 35720fb) — your commit was cherry-picked onto current main with your authorship preserved in git log. Your fix landed intact through two intervening merges to the same block (the generic pre-catalog guard from #85434 and the non-positive guard from #85507); the shared _model_name_suggests_stale_32k_underreport() predicate now covers Kimi + MiniMax at all three sites exactly as you wrote it. Thanks for covering the Nous-path _safe_ctx too, not just the step-1 cache — that's the half most fixes miss.

@teknium1 teknium1 closed this Aug 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P2 Medium — degraded but workaround exists provider/minimax MiniMax (Anthropic transport) sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-caching Sweeper risk: may break/degrade prompt caching or cache-key stability (invariant) sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants