-
Notifications
You must be signed in to change notification settings - Fork 677
Wrapped LLM as a garak generator #1382
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 5 commits
5f0b1af
a7d109b
6a54502
65645dc
094141d
7f20877
f2d730e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| garak.generators.llm | ||
| ========================== | ||
|
|
||
| .. automodule:: garak.generators.llm | ||
| :members: | ||
| :undoc-members: | ||
| :show-inheritance: |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,94 @@ | ||||||||||||||||||||||||||||||||||
| # SPDX-FileCopyrightText: Portions Copyright (c) 2025 NVIDIA CORPORATION & AFFILIATES. All rights reserved. | ||||||||||||||||||||||||||||||||||
| # SPDX-License-Identifier: Apache-2.0 | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| """LLM (simonw/llm) generator support""" | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| import logging | ||||||||||||||||||||||||||||||||||
| from typing import List, Union | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| import llm | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| from garak import _config | ||||||||||||||||||||||||||||||||||
| from garak.attempt import Message, Conversation | ||||||||||||||||||||||||||||||||||
| from garak.generators.base import Generator | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| class LLMGenerator(Generator): | ||||||||||||||||||||||||||||||||||
| """Class supporting simonw/llm-managed models | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| See https://pypi.org/project/llm/ and its provider plugins. | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| Calls model.prompt() with the prompt text and relays the response. Per-provider | ||||||||||||||||||||||||||||||||||
| options and API keys are all handled by `llm` (e.g., `llm keys set openai`). | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| Set --model_name to the `llm` model id or alias (e.g., "gpt-4o-mini", | ||||||||||||||||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||||||||||||||||
| "claude-3.5-haiku", or a local alias configured in `llm models`). | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| Explicitly, garak delegates the majority of responsibility here: | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| * the generator calls prompt() on the resolved `llm` model | ||||||||||||||||||||||||||||||||||
| * provider setup, auth, and model-specific options live in `llm` | ||||||||||||||||||||||||||||||||||
| * there's no support for chains; this is a direct LLM interface | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| Notes: | ||||||||||||||||||||||||||||||||||
| * Not all providers support all parameters (e.g., temperature, max_tokens). | ||||||||||||||||||||||||||||||||||
| We pass only non-None params; providers ignore what they don't support. | ||||||||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| DEFAULT_PARAMS = Generator.DEFAULT_PARAMS | { | ||||||||||||||||||||||||||||||||||
| "temperature": None, | ||||||||||||||||||||||||||||||||||
| "max_tokens": None, | ||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||
| "temperature": None, | |
| "max_tokens": None, |
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove, the system prompt is set via the run configuration and pass to generators as part of the prompt conversation.
| "system": None, |
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this might be better as lower case - that's how the tool is described
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we rename self.model to self.target to be consistent with overall garak nomenclature? llm supports both systems and models so the change fits this use-case fine too
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This does not grab out the full conversation. There is an existing helper function in the base class Generator._conversation_to_list() that will format the garak Conversation object as a list of dictionaries meeting the HuggingFace and OpenAI conversation list. Looking at how the llm library handles what it considers to be conversation I don't know if there is a way to load a prefilled history in a similar pattern to how chat completions APIs for other generators are working.
For best adoption, this generator should at least validate the conversation has at most one user and one system message to know if the prompt passed will be fully processed during inference.
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Current system prompt support in garak is tied to the conversation passed as part of prompt. The DEFAULT_PARAMS entry here should likely be removed in favor of extracting the system_prompt from the prompt via prompt.last_message("system"). That is if passing a conversation that includes the system message would not apply it.
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
None == False and all keys defined in DEFAULT_PARAMS will exist on self.
| if self.max_tokens is not None: | |
| prompt_kwargs["max_tokens"] = self.max_tokens | |
| if self.temperature is not None: | |
| prompt_kwargs["temperature"] = self.temperature | |
| if self.top_p is not None: | |
| prompt_kwargs["top_p"] = self.top_p | |
| if self.stop: | |
| prompt_kwargs["stop"] = self.stop | |
| if self.max_tokens: | |
| prompt_kwargs["max_tokens"] = self.max_tokens | |
| if self.temperature: | |
| prompt_kwargs["temperature"] = self.temperature | |
| if self.top_p: | |
| prompt_kwargs["top_p"] = self.top_p | |
| if self.stop: | |
| prompt_kwargs["stop"] = self.stop |
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we should land #1199 before landing this, and then move this PR to the deferred loading pattern |
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we should land #1199 before landing this, and then move this PR to the deferred loading pattern |
| Original file line number | Diff line number | Diff line change | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,128 @@ | ||||||||||||
| # SPDX-FileCopyrightText: Portions Copyright (c) 2025 NVIDIA CORPORATION & | ||||||||||||
| # AFFILIATES. All rights reserved. | ||||||||||||
|
Comment on lines
+1
to
+2
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||
| # SPDX-License-Identifier: Apache-2.0 | ||||||||||||
|
|
||||||||||||
| """Tests for simonw/llm-backed garak generator""" | ||||||||||||
|
|
||||||||||||
| import pytest | ||||||||||||
| from unittest.mock import MagicMock | ||||||||||||
|
|
||||||||||||
| from garak.attempt import Conversation, Turn, Message | ||||||||||||
| from garak._config import GarakSubConfig | ||||||||||||
|
|
||||||||||||
| # Adjust import path/module name to where you placed the wrapper | ||||||||||||
| from garak.generators.llm import LLMGenerator | ||||||||||||
|
|
||||||||||||
|
|
||||||||||||
| # ─── Helpers & Fixtures ───────────────────────────────────────────────── | ||||||||||||
|
|
||||||||||||
| class FakeResponse: | ||||||||||||
| """Minimal `llm` Response shim with .text()""" | ||||||||||||
| def __init__(self, txt: str): | ||||||||||||
| self._txt = txt | ||||||||||||
| def text(self) -> str: | ||||||||||||
| return self._txt | ||||||||||||
|
|
||||||||||||
|
|
||||||||||||
| class FakeModel: | ||||||||||||
| """Minimal `llm` model shim with .prompt()""" | ||||||||||||
| def __init__(self): | ||||||||||||
| self.calls = [] | ||||||||||||
| def prompt(self, prompt_text: str, **kwargs): | ||||||||||||
| self.calls.append((prompt_text, kwargs)) | ||||||||||||
| return FakeResponse("OK_FAKE") | ||||||||||||
|
|
||||||||||||
|
|
||||||||||||
| @pytest.fixture | ||||||||||||
| def cfg(): | ||||||||||||
| """Minimal garak sub-config; extend if you wire defaults via config.""" | ||||||||||||
| c = GarakSubConfig() | ||||||||||||
| c.generators = {} | ||||||||||||
| return c | ||||||||||||
|
|
||||||||||||
|
|
||||||||||||
| @pytest.fixture | ||||||||||||
| def fake_llm(monkeypatch): | ||||||||||||
| """ | ||||||||||||
| Patch llm.get_model to return a fresh FakeModel for each test. | ||||||||||||
| Return the FakeModel so tests can inspect call args. | ||||||||||||
| """ | ||||||||||||
| import llm | ||||||||||||
| model = FakeModel() | ||||||||||||
| monkeypatch.setattr(llm, "get_model", lambda *a, **k: model) | ||||||||||||
| return model | ||||||||||||
|
|
||||||||||||
|
|
||||||||||||
| # ─── Tests ────────────────────────────────────────────────────────────── | ||||||||||||
|
|
||||||||||||
| def test_instantiation_resolves_model(cfg, fake_llm): | ||||||||||||
| gen = LLMGenerator(name="my-alias", config_root=cfg) | ||||||||||||
| assert gen.name == "my-alias" | ||||||||||||
|
Comment on lines
+60
to
+61
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||
| assert hasattr(gen, "model") | ||||||||||||
|
||||||||||||
| assert "LLM (simonw/llm)" in gen.fullname | ||||||||||||
|
|
||||||||||||
|
|
||||||||||||
| def test_generate_returns_message(cfg, fake_llm): | ||||||||||||
| gen = LLMGenerator(name="alias", config_root=cfg) | ||||||||||||
|
|
||||||||||||
| conv = Conversation([Turn("user", Message(text="ping"))]) | ||||||||||||
|
||||||||||||
| conv = Conversation([Turn("user", Message(text="ping"))]) | |
| test_txt = "ping" | |
| conv = Conversation([Turn("user", Message(text=test_txt))]) |
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| assert prompt_text == "ping" | |
| assert prompt_text == test_txt |
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use vars for these values (and the checks later)
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
vars here. could do a list assignment / check likex,y = 1,2 for brevity
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's worth implementing the
_load_client()/_clear_client()pattern here to support parallelisation - seeopenai.OpenAICompatiblefor an example