Skip to content

Conversation

@DajanaV
Copy link
Collaborator

@DajanaV DajanaV commented Nov 17, 2025

Mirrored from ggml-org/llama.cpp#17330

I noticed the grammar README documentation was outdated. It stated that rule names must be 'dashed lowercase words', but when I looked at the examples like c.gbnf, I saw it uses dataType with an uppercase letter.

After checking the discussion in #7720, it's clear the parser actually supports much more flexibility than the docs suggested:

  • Uppercase and lowercase letters
  • Numbers
  • Both dashes and underscores

This PR updates the documentation to accurately reflect what the parser actually supports. The examples work fine - the docs just needed updating to match reality.

Changes:

  • Updated the 'Non-Terminals and Terminals' section to list all supported characters
  • Added examples showing the flexibility (dataType, UPPER-CASE, rule_123)

This should help people understand they have more flexibility when naming their grammar rules.

Fixes #7720

The documentation stated that non-terminal symbols must be 'dashed lowercase
words' like 'move' or 'check-mate', but the actual parser supports much more
flexibility. Rule names can include:
- Uppercase letters (e.g., dataType, UPPER-CASE)
- Numbers (e.g., rule123)
- Both dashes and underscores

This was discovered when c.gbnf used 'dataType' with an uppercase letter,
which works correctly despite the documentation saying otherwise. Updated
the docs to accurately describe the parser's capabilities rather than
restrict them.

Fixes #7720

Signed-off-by: Samaresh Kumar Singh <[email protected]>
@loci-agentic-ai
Copy link

Access the complete analysis in the LOCI Dashboard

Performance Analysis Summary

Based on the comprehensive analysis of PR #243 in the llama.cpp repository, this change represents a documentation-only modification with no impact on executable code or performance metrics.

Change Overview

The pull request updates the grammar README file (grammars/README.md) to correct outdated documentation about non-terminal naming rules. The change clarifies that grammar rule names support uppercase letters, lowercase letters, numbers, dashes, and underscores, expanding beyond the previously documented "dashed lowercase words" restriction.

Scope of Changes:

  • Single file modification: grammars/README.md
  • Net change: 0 lines (1 addition, 1 deletion)
  • Change type: Text clarification and example expansion

Performance Impact Assessment

Function-Level Analysis:

  • No functions modified or affected
  • No changes to Response Time or Throughput metrics
  • No impact on critical inference functions (llama_decode, llama_encode, llama_tokenize)
  • No changes to core modules (Model Processing, Token Processing, Memory Management, Batch Processing)

System Performance:

  • Tokens per second: No impact (no executable code changes)
  • Power consumption: No changes across any binaries
  • Memory usage: No modifications to memory management or KV cache systems
  • Inference pipeline: No alterations to tokenization or model processing paths

Technical Validation

The documentation update correctly aligns with the actual grammar parser implementation, which already supported the expanded naming conventions. The change improves developer experience by providing accurate guidance and concrete examples without affecting runtime behavior.

Risk Assessment:

  • No regression potential (documentation only)
  • No breaking changes to existing functionality
  • No impact on build process or dependencies

This change represents a pure documentation improvement that enhances accuracy and usability without any performance implications for the llama.cpp inference engine.

@DajanaV DajanaV force-pushed the main branch 3 times, most recently from f333350 to 9c4623f Compare November 18, 2025 09:10
@loci-dev loci-dev force-pushed the main branch 24 times, most recently from d2e6325 to 22143ca Compare November 24, 2025 14:09
@loci-dev loci-dev force-pushed the main branch 25 times, most recently from ad5ad9a to aaa8a85 Compare November 27, 2025 15:08
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