Skip to content

feat(llm): add configurable LLMCustomRule evaluator#397

Merged
e06084 merged 13 commits intoMigoXLab:devfrom
daniel5u:main
May 9, 2026
Merged

feat(llm): add configurable LLMCustomRule evaluator#397
e06084 merged 13 commits intoMigoXLab:devfrom
daniel5u:main

Conversation

@daniel5u
Copy link
Copy Markdown
Contributor

Summary

  • add built-in LLMCustomRule evaluator with structured custom_rule config
  • keep nested LLM config typed and isolated in Model.set_config_llm
  • add prompt-injection hardening in custom-rule system prompt
  • add tests for config parsing, message building, missing-field short-circuit, score mapping, and instance isolation
  • add docs entry and runnable .env example script and data

Validation

  • python -m pytest -q test/scripts/model/test_model_config_isolation.py test/scripts/model/llm/test_llm_custom_rule.py
  • python -m pytest -q test/scripts/model/llm/test_text_quality_v5.py
  • python -m pytest -q test/scripts/exec/test_cli.py
  • python examples/custom/run_llm_custom_rule_from_env.py (verified 1 GOOD plus 1 BAD on sample data)

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces the LLMCustomRule evaluator, which enables users to define custom LLM-based quality metrics via configuration. The changes include new Pydantic schemas for custom rules, logic for prompt construction and response parsing, and updates to the model configuration system to ensure deep copying and isolation of dynamic settings. Review feedback identifies a non-deterministic fallback when the model name is missing, a bug in the default exception name initialization, and an opportunity to simplify configuration dictionary creation using Pydantic's native methods.

if self.dynamic_config.model:
model_name = self.dynamic_config.model
else:
model_name = self.client.models.list().data[0].id
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.

high

Fetching the model list and picking the first available ID as a fallback is non-deterministic and inefficient. The first model returned by the API might not support chat completions (e.g., it could be an embedding or image model), which would cause a runtime error. Additionally, this adds an extra network request for every evaluation if the model is not specified in the config. It is recommended to either require the model field in the configuration or provide a sensible hardcoded default (e.g., gpt-4o-mini).

Comment thread dingo/model/llm/llm_custom_rule.py Outdated

attempts = 0
except_msg = ""
except_name = Exception.__class__.__name__
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.

medium

This initializes except_name to 'type' because Exception.__class__ refers to the type class. While this variable is likely to be overwritten in the exception handlers, it is better to use Exception.__name__ to correctly default to the string 'Exception' for clarity.

Suggested change
except_name = Exception.__class__.__name__
except_name = Exception.__name__

Comment thread dingo/model/model.py Outdated
Comment on lines +173 to +177
config_items = {
field_name: getattr(llm_config, field_name)
for field_name in llm_config.__class__.model_fields
}
config_items.update(llm_config.model_extra)
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.

medium

This logic can be simplified using dict(llm_config). In Pydantic v2, calling dict() on a model instance returns a shallow dictionary containing both the defined fields and any extra attributes (if extra='allow' is set), which achieves the goal of preserving nested Pydantic objects while including extra fields.

        config_items = dict(llm_config)

@daniel5u daniel5u changed the base branch from main to dev April 30, 2026 02:15
@e06084 e06084 merged commit 8ebe829 into MigoXLab:dev May 9, 2026
2 checks passed
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