feat(checks): add pydantic-settings for default model and config#2581
Conversation
Introduce GiskardChecksSettings backed by pydantic-settings so checks and scan can configure the default LLM/embedding models and other library options via GISKARD_CHECKS_* environment variables. - Wire get_default_generator/get_default_embedding_model to settings - Consolidate max_reported_failures and disable_rich_pretty into settings - Update integration tests and CI to use GISKARD_CHECKS_DEFAULT_MODEL - Add unit tests for settings resolution and precedence Co-authored-by: Kevin Messiaen <kevinmessiaen@users.noreply.github.com>
There was a problem hiding this comment.
Code Review
This pull request introduces pydantic-settings to manage configuration via environment variables and .env files, replacing direct os.getenv calls with a structured GiskardChecksSettings class. Feedback focuses on avoiding eager settings instantiation at import time in __init__.py, removing unnecessary caching via @lru_cache to keep settings dynamic, and refining the max_reported_failures validator to prevent booleans from being incorrectly treated as integers.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
- Revert import-time get_settings() for rich.pretty (use os.getenv) - Remove @lru_cache so settings stay responsive to env changes - Reject bool values in max_reported_failures validator Co-authored-by: Kevin Messiaen <kevinmessiaen@users.noreply.github.com>
- Unify disable_rich_pretty on get_settings() in __init__.py - Inline max_reported_failures lookup; move env var constant to settings - Remove unused set_default_embedding_model and runtime embedding override - Shorten max_reported_failures validator - Promote reset_default_generator fixture to package conftest Co-authored-by: Kevin Messiaen <kevinmessiaen@users.noreply.github.com>
davidberenstein1957
left a comment
There was a problem hiding this comment.
I had a minor remark about reasoning mode for model settings.
| _default_generator: BaseGenerator | None = None | ||
| _default_embedding_model: BaseEmbeddingModel | None = None | ||
|
|
||
| DEFAULT_MODEL = "openai/gpt-4o-mini" |
There was a problem hiding this comment.
should we also give control over reasonign modes?
There was a problem hiding this comment.
For now we don't really set reasoning mode globally. Each call side can set a value.
But long term we will probably have something like this:
EASY_TASK_MODEL = {"model": ..., "reasoning_effort": "low"}
HARD_TASK_MODEL = {"model": ..., "reasoning_effort": "high"}
Summary
Adds
pydantic-settingstogiskard-checksso library configuration can be driven by environment variables (or a.envfile), withset_default_generator()retained as a runtime override.Settings
New
GiskardChecksSettingsclass (GISKARD_CHECKS_*prefix):GISKARD_CHECKS_DEFAULT_MODELopenai/gpt-4o-miniGISKARD_CHECKS_DEFAULT_EMBEDDING_MODELtext-embedding-3-smallGISKARD_CHECKS_MAX_REPORTED_FAILURESGISKARD_CHECKS_DISABLE_RICH_PRETTYfalsePrecedence:
set_default_generator()/set_default_embedding_model()runtime overrides beat environment settings.Impact on scan
giskard-scanalready callsget_default_generator()for recommendations — it automatically picks upGISKARD_CHECKS_DEFAULT_MODELwith no scan code changes.CI / tests
GISKARD_CHECKS_DEFAULT_MODELandGISKARD_CHECKS_DEFAULT_EMBEDDING_MODELinstead ofTEST_MODEL.get_default_generator()instead of hardcoded model strings.Verification