Skip to content

Fix HuggingFaceProvider.generate() to apply chat template - #159

Open
Muhammad-Hashir-Code wants to merge 2 commits into
sugarlabs:mainfrom
Muhammad-Hashir-Code:fix/hf-provider-generate-chat-template
Open

Fix HuggingFaceProvider.generate() to apply chat template#159
Muhammad-Hashir-Code wants to merge 2 commits into
sugarlabs:mainfrom
Muhammad-Hashir-Code:fix/hf-provider-generate-chat-template

Conversation

@Muhammad-Hashir-Code

Copy link
Copy Markdown
Contributor

Summary

HuggingFaceProvider.generate() sends prompts to the model pipeline without applying the tokenizer's chat template. Since the models used by this provider (including the DEV_MODE default, SmolLM-135M-Instruct) are instruction-tuned, they are trained to expect chat-formatted input and do not reliably produce meaningful output from a raw, unformatted string. In practice this surfaces as empty or truncated responses on /ask-llm, and intermittently on /ask (which calls generate() internally via RAGAgent.run()).

This PR fixes generate() to apply the chat template consistently, by delegating to the already-correct chat() implementation.

Root Cause

BaseProvider establishes the correct pattern for generate():

# app/providers/base.py
def generate(self, prompt: str, params=None) -> str:
    """Generate text from a plain prompt by wrapping it as a user message."""
    return self.chat([{"role": "user", "content": prompt}], params)

HuggingFaceProvider.chat() correctly applies the chat template before generation:

# app/providers/huggingface.py — chat(), unchanged
full_prompt = self._pipeline.tokenizer.apply_chat_template(
    normalized, tokenize=False, add_generation_prompt=True,
)

However, HuggingFaceProvider overrides generate() with a separate implementation that calls the pipeline directly on the raw prompt, skipping the chat template entirely:

# app/providers/huggingface.py — generate(), before this PR
def generate(self, prompt: str, params=None) -> str:
    response = self._pipeline(
        prompt,  # raw string, no chat template applied
        max_new_tokens=params.max_new_tokens,
        ...
    )
    ...

For an instruction-tuned model, an unformatted prompt gives the model no clear signal for what to continue — it may emit an end-of-sequence token almost immediately (producing an empty string once the prompt is stripped from the output), or generate an incoherent/truncated continuation.

Call sites affected

Caller Path Impact
POST /ask-llm agent.provider.generate(question) Direct — reproduces reliably on short/simple questions
POST /ask RAGAgent.run() → provider.generate() (called twice per request) Indirect — same failure mode, masked somewhat by the retrieved context making prompts longer/more structured

Fix

HuggingFaceProvider.generate() now wraps the prompt as a single user message and delegates to self.chat(...), matching the pattern already established in BaseProvider:

def generate(self, prompt: str, params: Optional[GenerationParams] = None) -> str:
    """Generate text from a plain string prompt.
Wraps the prompt as a single user message and delegates to chat(),
so the model's chat template is applied consistently. Without this,
instruction-tuned models receive an unformatted prompt they were not
trained to continue from, which can produce empty or truncated output.
"""
return self.chat([{"role": "user", "content": prompt}], params)

This removes the duplicated pipeline-calling logic (~15 lines) and ensures generate() and chat() share a single, consistent code path for prompt formatting and generation.

Testing

Reproduced locally against DEV_MODE=1, DEV_MODEL_NAME=HuggingFaceTB/SmolLM-135M-Instruct, on an 8GB RAM / CPU-only machine.

Before fix

POST /ask-llm?question=What is Python?
{
  "answer": "",
  "user": "Admin Key",
  "quota": { "remaining": 99, "total": 100 }
}

After fix — same request

{
  "answer": "Python is a high-level, interpreted programming language that is widely used in various domains such as web development, scientific computing, data analysis, and more...",
  "user": "Admin Key",
  "quota": { "remaining": 100, "total": 100 }
}

Regression check — /ask (RAG pipeline)

Verified /ask continues to return complete, non-empty responses after the change, confirming no regression in the retrieval path that also relies on generate().

Scope

This change is isolated to HuggingFaceProvider.generate() in app/providers/huggingface.py. It does not modify chat(), the RAG pipeline logic in app/ai.py, route handlers in app/routes/api.py, or any other provider. No new dependencies, no config changes, no API contract changes.

Checklist

  • Reproduced the bug locally before writing the fix
  • Verified the fix resolves the reported symptom (/ask-llm empty answers)
  • Verified no regression on /ask
  • No new dependencies or config changes introduced

generate() was sending raw, unformatted prompts directly to the model pipeline, bypassing the chat template that instruction-tuned models expect. This caused empty or truncated responses on endpoints like /ask-llm. Fixed by delegating to chat(), matching the pattern already used in BaseProvider.
@Muhammad-Hashir-Code

Copy link
Copy Markdown
Contributor Author

Hi @chimosky and @walterbender, I am checking in on this pull request to see if you have had a chance to review it.

Please let me know if there are any adjustments, code formatting updates, or tests you would like me to add before this is ready for approval.

Thanks for your time!

@Noaman-Akhtar

Copy link
Copy Markdown
Contributor

Hey @Muhammad-Hashir-Code I tested this locally with SmolLM-135M-Instruct on both main and this branch.
On main, What is Python? returned an empty response . But other prompts like "Explain a Python loop in one sentence", did return responses, although they were often incomplete, unrelated, or too long. So the old behavior seems prompt dependent and unreliable, rather than always returning empty responses.
On this branch, the same prompts returned non-empty and slightly better-formatted responses, so applying the chat template does seem to help.
My only concern is that generate() now always uses apply_chat_template(). Models without a chat template may fail because of this.

@Muhammad-Hashir-Code

Muhammad-Hashir-Code commented Aug 14, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for testing this thoroughly, @Noaman-Akhtar appreciate the independent verification, and good catch on the fallback concern.

You're right that apply_chat_template() will fail for models that don't have a chat template defined on their tokenizer. I can add a check before calling it, something like:

def generate(self, prompt: str, params=None) -> str:
    if getattr(self._pipeline.tokenizer, "chat_template", None):
        return self.chat([{"role": "user", "content": prompt}], params)
    # fall back to the original raw-prompt behavior for models without a chat template
    ...

This way, models with a chat template get the fix, while models without one keep working exactly as before.

Worth noting too: all the models currently specified in .example.env/README for HuggingFaceProvider (DEV_MODEL_NAME=SmolLM-135M-Instruct, PROD_MODEL_NAME/DEFAULT_MODEL=Qwen2-1.5B-Instruct) are instruction-tuned and do have chat templates, so anyone testing with the project's documented setup should be fully covered. The fallback would only matter in the rarer case where someone swaps in a different, non-instruction-tuned model via /change-model or a custom config but it's worth handling gracefully either way.

@Noaman-Akhtar

Copy link
Copy Markdown
Contributor

@Muhammad-Hashir-Code Yes, a fallback would be useful. We want to keep the Hugging Face provider flexible so users can configure and use different models of their choice .

generate() now checks for a chat_template on the tokenizer before delegating to chat(). If none is present, it falls back to the original raw-prompt pipeline call, preserving compatibility with base models.
@Muhammad-Hashir-Code

Copy link
Copy Markdown
Contributor Author

@Noaman-Akhtar Just pushed a fix generate() now checks for a chat_template on the tokenizer before delegating to chat(), and falls back to the original raw-prompt behavior if none is present. Let me know if this addresses your concern.

@Noaman-Akhtar

Copy link
Copy Markdown
Contributor

LGTM @mebinthattil , @chimosky

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.

2 participants