Skip to content

fix: broaden llama.cpp grammar error classifier to match newer builds - #76152

Open
gonzalofrancoceballos wants to merge 1 commit into
NousResearch:mainfrom
gonzalofrancoceballos:fix/llama-cpp-grammar-classifier-broader-match
Open

fix: broaden llama.cpp grammar error classifier to match newer builds#76152
gonzalofrancoceballos wants to merge 1 commit into
NousResearch:mainfrom
gonzalofrancoceballos:fix/llama-cpp-grammar-classifier-broader-match

Conversation

@gonzalofrancoceballos

Copy link
Copy Markdown

Problem

The llama.cpp grammar error classifier in agent/error_classifier.py only recognizes older error phrasing ("error parsing grammar"), but newer llama.cpp builds return "Failed to initialize samplers: failed to parse grammar". When the classifier fails to match, Hermes skips the grammar recovery path (stripping pattern/format keywords from tool schemas and retrying locally) and falls directly to a cloud fallback provider.

This is especially impactful for users running local inference via custom providers (llama.cpp) with MCP servers whose tool schemas include regex patterns like "\\d{4}-\\d{2}-\\d{2}".

Reproduction

# Local llama.cpp server returns HTTP 400:
curl http://localhost:8080/v1/chat/completions -H "Content-Type: application/json" -d '{
  "model": "qwen3-80b-next-q4_k_m",
  "messages": [{"role":"user","content":"test"}],
  "tools": [{"type":"function","function":{"name":"f","parameters":{"type":"object","properties":{"d":{"type":"string","pattern":"\\d{4}-\\d{2}-\\d{2}"}}}}}],
  "max_tokens": 50
}'
# → {"error":{"message":"Failed to initialize samplers: failed to parse grammar"}}

In Hermes, this error passes through the classifier unrecognized, triggering cloud fallback instead of the schema-stripping recovery.

Fix

Added "failed to parse grammar" to the match list in the classifier condition, alongside the existing "error parsing grammar" and "unable to generate parser" checks.

Changes

  • agent/error_classifier.py: Added "failed to parse grammar" in error_msg condition
  • tests/agent/test_error_classifier.py: Added test_llama_cpp_failed_to_parse_grammar test case

Testing

pytest tests/agent/test_error_classifier.py::TestClassifyApiError::test_llama_cpp_failed_to_parse_grammar
# PASSED
pytest tests/agent/test_error_classifier.py::TestClassifyApiError::test_llama_cpp_grammar_parse_error
# PASSED (existing test still passes)

@teknium1

teknium1 commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

Thanks for the focused llama.cpp recovery fix. Current main recognizes the older grammar phrases at agent/error_classifier.py:780-795, but not "failed to parse grammar"; unmatched HTTP 400s continue into _classify_400 at agent/error_classifier.py:1120 and can become fallback-eligible format errors at agent/error_classifier.py:1443-1448. The existing recovery branch at agent/conversation_loop.py:4079-4104 already performs the intended one-shot schema strip when this classifier reason is returned.

No blocking issues found. The added predicate is narrow, preserves the HTTP-400 guard, and the PR's tests cover both the newly reported wording and the existing older parser wording.

Automated hermes-sweeper review.

@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 duplicate This issue or pull request already exists labels Aug 1, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Duplicate of #67349: both add the same failed-to-parse-grammar classifier recovery; #67349 also covers status-less streaming errors and the related schema-sanitizer path.

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users labels Aug 1, 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 reported classifier gap by matching the newer llama.cpp phrase "failed to parse grammar" so HTTP 400 responses enter the existing schema-stripping recovery path. #51527 and #76152 make the same one-line classifier change, with regression coverage differing only in test placement and the additional legacy-wording assertion in #76152.

Related pull requests

  • #51527 [closed] duplicate — (+13/-1) — duplicate implementation, already closed: adds the same narrow HTTP-400 predicate for "failed to parse grammar" and a regression test. It remains relevant as prior implementation evidence, although the supplied context does not establish why it was closed.
  • #76152 related — (+21/-1) — keep open with a salvage path: adds the missing phrase under the existing HTTP-400 guard and tests both the new wording and an older parser wording. This agrees with the maintainer-bot keep_open review; the contributor discussion comment calls it a duplicate of #67349, but no #67349 diff, state, or verification verdict is supplied to substantiate that relationship or its claimed broader coverage.

Duplicates

#51527 and #76152 are substantively duplicate implementations of the same classifier predicate; #51527 is already closed. #67349 is only a contributor-reported possible duplicate and cannot be placed in a verified duplicate chain from the supplied evidence.

Suggested consolidation

Keep #76152 open with a salvage path for its focused classifier predicate and regression coverage, consistent with the maintainer-bot keep_open review. Leave #51527 closed as the redundant prior implementation; before closing #76152 as a duplicate of #67349, verify #67349's current state and diff, including the claimed status-less streaming and schema-sanitizer coverage.

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 Dup51527 ["PRs duplicating each other"]
        P51527["PR #51527 (closed)"]
        P76152["PR #76152 (open)"]
    end
    class P51527 closed
    class P76152 open
    class P76152 target
    click P51527 "https://github.com/NousResearch/hermes-agent/pull/51527"
    click P76152 "https://github.com/NousResearch/hermes-agent/pull/76152"
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 (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: 4 kB of PR diffs, 4 kB of issue/PR text, <1 kB of discussion (1 comments), 0 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.

The error message varies across llama.cpp versions:
- Older: "error parsing grammar: unknown escape at \\d"
- Newer: "Failed to initialize samplers: failed to parse grammar"

Only the older phrasing was recognized, causing Hermes to fall back
to cloud providers instead of stripping pattern/format keywords and
retrying locally. Added "failed to parse grammar" to the match list
and updated the classifier comment to document the newer variant.

Test: added test_llama_cpp_failed_to_parse_grammar covering the new phrase.
@gonzalofrancoceballos
gonzalofrancoceballos force-pushed the fix/llama-cpp-grammar-classifier-broader-match branch from a92b2cb to 8d0b090 Compare August 18, 2026 17:29
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 duplicate This issue or pull request already exists P2 Medium — degraded but workaround exists sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users 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