Skip to content

feat(config): add config validation to tests and tooling (closes #937) - #1023

Open
arcgod-design wants to merge 1 commit into
imDarshanGK:mainfrom
arcgod-design:feat/issue-937-config-validation
Open

feat(config): add config validation to tests and tooling (closes #937)#1023
arcgod-design wants to merge 1 commit into
imDarshanGK:mainfrom
arcgod-design:feat/issue-937-config-validation

Conversation

@arcgod-design

Copy link
Copy Markdown

What

Adds a zero-dependency config-validation rules engine to backend/utils/config_validator.py, with tests and CLI tooling — closing issue #937.

The existing AppSettings Pydantic model (backend/models/schemas.py:124) validates individual field types (e.g., temperature: float = 0.7 accepts any float) but does not enforce cross-field constraints or environment-variable sanity:

  • rag_chunk_overlap must be strictly less than rag_chunk_size (otherwise every chunk after the first is identical to its predecessor — wasteful and confusing for the user).
  • OLLAMA_HOST unset → backend silently falls back to localhost:11434, hardly noticeable in production.
  • CORS_ORIGINS='*' is silently accepted.
  • theme='rose-gold' is silently coerced by the UI.

This PR adds a lightweight rules engine that surfaces all of these at startup or in unit tests without requiring Pydantic field-validator boilerplate.

Why

  • PR reviews catch bad config by accident: a typo landing at startup costs hours of debugging. Validation surfaced up-front means reviewer eyes on root causes, not on stdout.
  • Pydantic is the wrong layer for cross-field checks: it works, but the constraint leaks into the data model.
  • Env-var checks shouldn't belong in app.py: the lifespan handler is already cluttered and any silent skip there gets noticed only when a customer reports it.
  • Tests need it too: contributors writing new tests against AppSettings would benefit from one-call validate(config) instead of re-implementing per-PR.

What changed

Backend utility — backend/utils/config_validator.py (NEW)

Public surface:

@dataclass(frozen=True)
class ValidationIssue:
    severity: str   # one of SEVERITY_ERROR / WARN / INFO
    location: str
    message: str

@dataclass
class ValidationRule:
    name: str
    fn: Callable[[dict], list[ValidationIssue]]

def validate(config, rules=None) -> list[ValidationIssue]
def has_errors(issues) -> bool
def has_warnings(issues) -> bool
def summarise(issues) -> dict   # {total, errors, warnings, infos, issues: [...]}
def load_rules_from_module(spec_str) -> list[ValidationRule]

Rules are wrapped in try/except in validate() — a buggy rule surfaces as a single ERROR issue with its exception string, never by crashing the run.

DEFAULT_RULES covers the LocalMind AppSettings surface plus cross-field and env checks:

Rule Severity What it catches
model_name ERROR default_model empty / non-string
language_supported WARN default_language not in (en, ja, es, fr, de)
temperature_range ERROR temperature outside [0.0, 2.0] (inclusive)
max_history_turns ERROR / WARN < 1 error; > 100 warn (O(N) memory cost); bool rejected
rag_top_k ERROR outside [1, 64]
rag_chunk_overlap ERROR not int in [0, 10000]; strictly less than rag_chunk_size
theme_value WARN not in (light, dark, system, auto)
ollama_host_env INFO OLLAMA_HOST unset → backend falls back to localhost
cors_origins_env ERROR / WARN wildcard '*' rejected; missing-scheme warns
chunk_tuning_consistency INFO chunk_size % overlap == 0 → adjacent chunks share too much content

Tooling — scripts/config_validator_cli.py (NEW)

validate <config.json>   Run validate(); print summarise(); exit non-zero on errors.
selfcheck                Run the ruleset against a canary bad config covering ≥5 rules.

Exit codes 0/1/2 per repo convention. UTF-8 stdout rewrapped so non-ASCII glyphs in messages (e.g. in "must be ≥ 1") don't crash the Windows cp1252 console — bug noticed during self-check.

Example:

$ python scripts/config_validator_cli.py validate malformed.json
{
  "total": 8, "errors": 5, "warnings": 2, "infos": 1,
  "issues": [
    {"severity": "ERROR", "location": "settings.default_model", "message": "default_model must be a non-empty string (got '')"},
    ...
  ],
  "input": "malformed.json"
}
Exit: 1

Tests — backend/tests/test_config_validator.py (NEW, 46 cases)

Class Cases Coverage
TestValidationIssue 3 construction, invalid-severity raises, to_dict shape
TestValidateDriver 4 empty rules, no-issue pass, exception surfaces as ERROR, end-to-end clean
TestDefaultModelRule / TestLanguageRule / TestTemperatureRule / TestMaxHistoryTurnsRule / TestRagTopKRule / TestRagOverlapRule / TestChunkTuningConsistency / TestThemeRule 22 clean path, boundary, mutant path per rule; bool rejected for int fields
TestEnvRules 5 OLLAMA_HOST unset → INFO, explicit → silent; CORS wildcard rejected, missing-scheme warned, well-formed ok
TestSummarise 2 empty + mixed counts
TestAggregateHelpers 4 has_errors / has_warnings shape
TestLoadRulesFromModule 3 round-trip via stub module on sys.path; missing RULES defaults to []; wrong type raises
TestEndToEnd 1 default LocalMind config → 0 errors

Verification

$ cd backend && pytest tests/test_config_validator.py -v
============================= 46 passed in 0.49s ==============================

$ ruff check backend/utils/config_validator.py \
           backend/tests/test_config_validator.py scripts/config_validator_cli.py
All checks passed!

$ ruff format --check <same files>
3 files already formatted

$ python scripts/config_validator_cli.py selfcheck
{...total: 8, errors: 5, warnings: 2, infos: 1...}
Exit: 0

Acceptance criteria

  • Implement the change in the tests and tooling.
  • Add or update tests, docs, or validation for the touched path.

Closes #937. With this PR, the entire localmind SSoC-26 backlog of issues #928-#937 is now in review (8 PRs open).

…rshanGK#937)

Adds a zero-dependency rules engine at
backend/utils/config_validator.py that surfaces invalid LocalMind
app-config at startup or in tests:

- ValidationIssue (frozen): severity, location, message —
  severity restricted to {ERROR, WARN, INFO}.
- ValidationRule: name + fn(config) -> [ValidationIssue].
- validate(config, rules=None): runs DEFAULT_RULES (or caller
  supplied list) catching exceptions so a buggy rule doesn't crash
  the report.
- has_errors / has_warnings / summarise helpers.
- load_rules_from_module(spec) imports a 'module' and pulls its
  RULES list for reuse-extension without monkey-patching.

Default ruleset covers AppSettings (Pydantic only validates single
field types; cross-field/system context checks go here):
  - model_name (non-empty)
  - language_supported ({en,ja,es,fr,de}; warns for others)
  - temperature_range (0..2 inclusivity)
  - max_history_turns (>=1; warns >100)
  - rag_top_k (1..64)
  - rag_chunk_overlap strictly < rag_chunk_size (else no new content
    in any chunk — wasteful and confusing)
  - theme_value ({light,dark,system,auto})
  - OLLAMA_HOST env (INFO if unset)
  - CORS_ORIGINS env ('*' rejected; missing scheme warned)
  - chunk_tuning_consistency (INFO when chunk_size % overlap == 0;
    retrieval returns near-identical chunks for adjacent queries)

New tooling: scripts/config_validator_cli.py:
  validate <config.json> — validates and prints JSON summary with
    all issues + counts; exit non-zero if errors.
  selfcheck              — runs the ruleset against a canary bad
    config with ≥5 obvious errors to smoke-test the validator itself.
  Exit codes 0/1/2. UTF-8 stdout wrapped so non-ASCII glyphs (the '≥'
  in 'must be ≥ 1' messages) don't crash Windows cp1252 console.

New tests: backend/tests/test_config_validator.py — 46 pytest cases:
- TestValidationIssue (3): construction, invalid severity raises,
  to_dict shape.
- TestValidateDriver (4): empty rules, no issue passes, rule exception
  surfaces as ERROR without crashing, end-to-end clean config.
- TestDefaultModelRule / TestLanguageRule / TestTemperatureRule /
  TestMaxHistoryTurnsRule / TestRagTopKRule / TestRagOverlapRule /
  TestChunkTuningConsistency / TestThemeRule (each): clean path,
  boundary, mutant path per the rule's invariants. Bool rejected
  for int fields (Python's True isinstance int gotcha).
- TestEnvRules (5): OLLAMA_HOST unset -> INFO, explicit -> silent;
  CORS wildcard rejected, missing-scheme warned, well-formed ok.
- TestSummarise (2): empty + mixed counts.
- TestAggregateHelpers (4): has_errors / has_warnings shape.
- TestLoadRulesFromModule (3): round-trip via stub module on sys.path,
  missing RULES default [], wrong-type raises.
- TestEndToEnd (1): default LocalMind config -> 0 errors.

Acceptance criteria from issue imDarshanGK#937:
  [x] Implement the change in the tests and tooling
  [x] Add or update tests, docs, or validation for the touched path
@vercel

vercel Bot commented Jul 24, 2026

Copy link
Copy Markdown

@arcgod-design is attempting to deploy a commit to the Darshan's projects Team on Vercel.

A member of the Team first needs to authorize it.

@imDarshanGK

Copy link
Copy Markdown
Owner

@arcgod-design
Please add a short video demo showcasing your key features for review.

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.

Add config validation to tests and tooling

3 participants